mom5: remove access-gtracers and type variants by harshula · Pull Request #227 · ACCESS-NRI/access-spack-packages

harshula · 2025-04-10T15:46:00Z

Closes #183

aidanheerdegen · 2025-04-10T23:52:10Z

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.

dougiesquire

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

dougiesquire · 2025-04-16T00:29:18Z

support building with legacy WOMBAT, which I think requires adding another version

Thinking about this more, I think perhaps a legacy_BGC variant might be better. I'll play around with this

harshula · 2025-04-16T00:41:56Z

Hi @aidanheerdegen , Do you still want #183 (comment) ?

aidanheerdegen · 2025-04-16T10:02:54Z

Hi @aidanheerdegen , Do you still want #183 (comment) ?

I think what I said still applies, unless there is some new information?

dougiesquire · 2025-04-16T10:06:58Z

Hi @aidanheerdegen , Do you still want #183 (comment) ?

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...

aidanheerdegen · 2025-04-16T10:15:58Z

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 ...

dougiesquire · 2025-04-16T10:26:24Z

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...

dougiesquire · 2025-04-21T23:52:55Z

Hi @dougiesquire , You can use spack-packages tag dev-2025.04.002. It only contains changes to the MOM5 SPR.

@harshula, without corresponding changes to the access-esm1p5, access-esm1p6 and access-om2-bgc SPRs, I can only test ACCESS-OM2.

But for ACCESS-OM2, spack-packages dev-2025.04.002 gives the same answers as my test branch as expected:

Model	Prerelease PR	Configuration test PR	Historical reproducibility
ACCESS-OM2	ACCESS-NRI/ACCESS-OM2#102	`dev-1deg_jra55_ryf` `dougiesquire/1deg_jra55_ryf_wombatlite`	❌ see here ❌ see here

Regarding my question from yesterday, are you happy with the string legacy-access-om2-bgc?

It's not ideal, but I guess so... To me, a legacy_bgc variant for the access-om2 version is clearer, but I think you don't like that option? Also, you told me on Zulip that we couldn't assume there is a Spack version specified in the spec. Is that not what you're now doing or am I misunderstanding?

harshula · 2025-04-22T00:48:06Z

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 · 2025-04-23T04:22:23Z

Hi @dougiesquire & @penguian , I've marked this PR Ready for Review. I'm just running build tests of access-om2 at the moment.

harshula · 2025-04-23T04:51:47Z

Hi @dougiesquire , Completed successful builds of ACCESS-OM2 (mom5@git.dev_2024.08.14=access-om2) and ACCESS-OM-BGC (mom5@git.dev-2025.04.001=legacy-access-om2-bgc) in a Docker container.

Can you please do your test runs on spack-packages tag dev-2025.04.004? Thanks!

dougiesquire · 2025-04-23T06:42:12Z

Testing with:

spack-packages tag dev-2025.04.004
MOM5 tag dev-2025.04.001

Model	Prerelease PR	Configuration test PR	Historical reproducibility
ACCESS-OM2	ACCESS-NRI/ACCESS-OM2#102	`dev-1deg_jra55_ryf` `dougiesquire/1deg_jra55_ryf_wombatlite`	❌ see here ❌ see here
ACCESS-OM2-BGC (legacy)	ACCESS-NRI/ACCESS-OM2-BGC#22	`dev-1deg_jra55_ryf_bgc`	✅ see here
ACCESS-ESM1.5	ACCESS-NRI/ACCESS-ESM1.5#35	`dev-preindustrial+concentrations`	✅ see here
ACCESS-ESM1.6	ACCESS-NRI/ACCESS-ESM1.6#79	`dev-preindustrial+concentrations`	✅ see here

As above, I think the answer changes to the ACCESS-OM2 configurations are expected.

dougiesquire

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:

harshula · 2025-04-23T14:17:44Z

Also, I'm curious about the planned order for merging things.

This goes first. Then we update relevant MDRs (spack.yaml). Then remaining PRs.

dougiesquire · 2025-04-23T23:16:04Z

Also, I'm curious about the planned order for merging things.

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 development branch. E.g. spack install access-om2 fails.

Is there a reason why we can't merge the MOM5 PR first? Are those changes incompatible with the current MOM5 SPR on main?

harshula · 2025-04-24T02:12:53Z

Okay, just noting that the changes in this PR won't work by default

Hi @dougiesquire , Which changes in this PR are you referring to?

dougiesquire · 2025-04-24T02:21:15Z

Okay, just noting that the changes in this PR won't work by default

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 development branch. Please just ignore me.

* 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.

penguian · 2025-04-30T01:16:48Z

Looks OK to me.

penguian

Approved, as discussed previously.

harshula requested review from aidanheerdegen and dougiesquire April 10, 2025 15:46

harshula self-assigned this Apr 10, 2025

harshula added this to ACCESS-OM2 Spack and ACCESS-ESM1.6 Apr 10, 2025

harshula requested a review from penguian April 10, 2025 15:51

harshula force-pushed the development branch from 4a8a15f to 463d989 Compare April 11, 2025 04:57

dougiesquire requested changes Apr 15, 2025

View reviewed changes

harshula force-pushed the development branch 2 times, most recently from e32b4fa to 4ef72be Compare April 16, 2025 05:20

dougiesquire requested changes Apr 16, 2025

View reviewed changes

Comment thread packages/mom5/package.py Outdated

harshula force-pushed the development branch 3 times, most recently from d303a35 to 2078d05 Compare April 16, 2025 05:41

dougiesquire requested changes Apr 16, 2025

View reviewed changes

Comment thread packages/mom5/package.py Outdated

This was referenced Apr 16, 2025

Test MOM5 spack-package changes ACCESS-NRI/ACCESS-OM2#102

Closed

Test MOM5 spack-package changes ACCESS-NRI/ACCESS-OM2-BGC#22

Closed

harshula force-pushed the development branch from 2078d05 to 3502f41 Compare April 16, 2025 06:33

This was referenced Apr 16, 2025

Test MOM5 spack-package changes ACCESS-NRI/ACCESS-ESM1.5#35

Closed

Test MOM5 spack-package changes ACCESS-NRI/ACCESS-ESM1.6#79

Closed

harshula force-pushed the development branch from 3502f41 to d6b875b Compare April 16, 2025 09:14

dougiesquire mentioned this pull request Apr 16, 2025

1deg_jra55_ryf: test MOM5 spack-package changes ACCESS-NRI/access-om2-configs#186

Closed

harshula force-pushed the development branch from 15891dd to 57e1173 Compare April 16, 2025 10:33

harshula force-pushed the development branch 4 times, most recently from 9ced071 to 8617192 Compare April 23, 2025 03:27

harshula requested review from dougiesquire and removed request for aidanheerdegen April 23, 2025 04:20

harshula marked this pull request as ready for review April 23, 2025 04:21

harshula force-pushed the development branch from 86acbb8 to 9386881 Compare April 23, 2025 04:45

dougiesquire reviewed Apr 23, 2025

View reviewed changes

Comment thread packages/access-om2-bgc/package.py Outdated

harshula force-pushed the development branch from 9386881 to f7ea045 Compare April 23, 2025 05:00

dougiesquire reviewed Apr 23, 2025

View reviewed changes

Comment thread packages/mom5/package.py

Comment thread packages/mom5/package.py

harshula force-pushed the development branch from f7ea045 to 775f701 Compare April 23, 2025 14:23

harshula and others added 2 commits April 24, 2025 14:19

SBR: Remove variant type and generic-tracers from mom5 specs

bd90a3c

harshula force-pushed the development branch from 775f701 to bd90a3c Compare April 24, 2025 04:20

penguian approved these changes Apr 30, 2025

View reviewed changes

dougiesquire approved these changes Apr 30, 2025

View reviewed changes

harshula merged commit b7c88c7 into main Apr 30, 2025

github-project-automation Bot moved this to Done in ACCESS-ESM1.6 Apr 30, 2025

github-project-automation Bot moved this to Done in ACCESS-OM2 Spack Apr 30, 2025

Conversation

harshula commented Apr 10, 2025

Uh oh!

aidanheerdegen commented Apr 10, 2025

Uh oh!

dougiesquire left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dougiesquire commented Apr 16, 2025

Uh oh!

harshula commented Apr 16, 2025

Uh oh!

Uh oh!

Uh oh!

aidanheerdegen commented Apr 16, 2025

Uh oh!

dougiesquire commented Apr 16, 2025

Uh oh!

aidanheerdegen commented Apr 16, 2025

Uh oh!

dougiesquire commented Apr 16, 2025

Uh oh!

dougiesquire commented Apr 21, 2025

Uh oh!

harshula commented Apr 22, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

harshula commented Apr 23, 2025

Uh oh!

harshula commented Apr 23, 2025

Uh oh!

Uh oh!

dougiesquire commented Apr 23, 2025

Uh oh!

dougiesquire left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

harshula commented Apr 23, 2025

Uh oh!

dougiesquire commented Apr 23, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

harshula commented Apr 24, 2025

Uh oh!

dougiesquire commented Apr 24, 2025

Uh oh!

penguian commented Apr 30, 2025

Uh oh!

penguian left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

dougiesquire left a comment •

edited

Loading

harshula commented Apr 22, 2025 •

edited

Loading

dougiesquire left a comment •

edited

Loading

dougiesquire commented Apr 23, 2025 •

edited

Loading