mom5: remove access-gtracers and type variants #227
Conversation
|
If you don't mind I'd prefer @dougiesquire and @penguian to review as they're more familiar with the requirements so I think would be better placed to do this. |
There was a problem hiding this comment.
Thanks @harshula. Looks good, but I'll do some testing now.
In the meantime, I think we decided to support building with legacy WOMBAT, which I think requires adding another version:
| version | branch | type | access-gtracers |
|---|---|---|---|
| access-om2-bgc-legacy | master | ACCESS-OM-BGC | F |
Looking at the MOM5 code, I think supporting this might require code changes since we've moved to spackified gtracers.
ADDED: I think we'll also need to talk about what to do with FMS for this version
Thinking about this more, I think perhaps a |
|
Hi @aidanheerdegen , Do you still want #183 (comment) ? |
harshula
force-pushed
the
development
branch
2 times, most recently
from
e32b4fa to
4ef72be
Compare
harshula
force-pushed
the
development
branch
3 times, most recently
from
d303a35 to
2078d05
Compare
I think what I said still applies, unless there is some new information? |
Oh, I actually misread your original comment @aidanheerdegen. I thought you were saying we should support building with legacy BGC, but now I see you're saying we should only add that if/when someone needs it... |
Yeah. Now I'm concerned this has created a whole bunch of work ... |
In my opinion, I think we're pretty much there. I think I have everything working (currently testing with prereleases and will report back), but @harshula is just working on making the SPR implementation a little better. @harshula may disagree about how close we are though... |
@harshula, without corresponding changes to the But for ACCESS-OM2,
It's not ideal, but I guess so... To me, a |
|
Hi @dougiesquire , There isn't a documented way to extract the version in a consistent way. I had to do a lot of debugging last week and reading source code to discover that the GitVersion class contained a member variable containing the non-git component version string. |
harshula
force-pushed
the
development
branch
4 times, most recently
from
9ced071 to
8617192
Compare
harshula
requested review from
dougiesquire
and removed request for
aidanheerdegen
|
Hi @dougiesquire & @penguian , I've marked this PR Ready for Review. I'm just running build tests of |
|
Hi @dougiesquire , Completed successful builds of ACCESS-OM2 ( Can you please do your test runs on |
|
Testing with:
As above, I think the answer changes to the ACCESS-OM2 configurations are expected. |
There was a problem hiding this comment.
Thanks @harshula, looking good to me. A couple of questions/comments below.
Also, I'm curious about the planned order for merging things. Would it be best to get the following merged before merging this PR:
This goes first. Then we update relevant MDRs (spack.yaml). Then remaining PRs. |
Okay, just noting that the changes in this PR won't work by default since they require the changes in the MOM5 Is there a reason why we can't merge the MOM5 PR first? Are those changes incompatible with the current MOM5 SPR on |
Hi @dougiesquire , Which changes in this PR are you referring to? |
Ah right yeah sorry - the MOM5 SPR already didn't work by default. I.e. even before this PR the MOM5 SPR relied on changes in the MOM5 |
* The version will determine type and whether gtracers is enabled:
"access-om2": {"type": "ACCESS-OM", "gtracers": True},
"legacy-access-om2-bgc": {"type": "ACCESS-OM-BGC", "gtracers": False},
"access-esm1.5": {"type": "ACCESS-CM", "gtracers": False},
"access-esm1.6": {"type": "ACCESS-ESM", "gtracers": True},
* Use GitVersion class' ref_version member to extract the version.
* Use explicit versions for clarity. No more "else" case.
* Thanks to Dougie Squire and Paul Leopardi for their help.
|
Looks OK to me. |
Closes #183