Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Pass salt flux restoring/correction to generic tracers #390

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

dougiesquire
Copy link
Collaborator

See #389 for context and details.

Reiterating from that issue:
These changes should not alter the current behaviour of MOM5 if the generic_tracer code in the local ocean_shared directory is used. However, if an external generic_tracer codebase is used (e.g. NOAA-GFDL/ocean_BGC), that code will need to include the new generic_tracer::generic_tracer_update_from_coupler routine. There is currently an open PR to add this routine to NOAA-GFDL/ocean_BGC.

Closes #389

@dougiesquire
Copy link
Collaborator Author

I don't have permission to request a review. @russfiedler, @aidanheerdegen, @aekiss, anyone pleeease?

@dougiesquire
Copy link
Collaborator Author

@nikizadehgfdl might you have time to review this? We're struggling to find reviewers with the necessary background knowledge of generic tracers and your eyes would be much appreciated.

These are the changes required to allow NOAA-GFDL/ocean_BGC#35

@dougiesquire
Copy link
Collaborator Author

Is there anyone out there that would be able to review?

@aekiss
Copy link
Collaborator

aekiss commented Aug 16, 2024

Sorry, I don't know nearly enough of the nuts and bolts to give a meaningful review.

@aidanheerdegen
Copy link
Contributor

@marshallward I've pinged you for a review, but if this isn't your thing feel free to rope in someone from GFDL who might know more.

THANKS!

@marshallward
Copy link
Collaborator

Definitely not my thing, but I will see if anyone here has the time to look. Otherwise, I'll try to make sense of it.

@aidanheerdegen
Copy link
Contributor

Definitely not my thing, but I will see if anyone here has the time to look. Otherwise, I'll try to make sense of it.

Thanks @marshallward

@theresa-morrison
Copy link

@dougiesquire I took a look at this, but I am much more familiar with MOM6. I can follow the logic here and it seems to be doing what you laid out in the issuse. I can't do any testing but I can provide a review if looking at the code is sufficient for you.

@aidanheerdegen
Copy link
Contributor

@dougiesquire I took a look at this, but I am much more familiar with MOM6. I can follow the logic here and it seems to be doing what you laid out in the issuse. I can't do any testing but I can provide a review if looking at the code is sufficient for you.

@dougiesquire is on parental leave, so in case he doesn't see this, yes please! Dougie has done a lot of testing, but I think he really wanted a sanity check to make sure there wasn't anything obvious he hadn't taken into account.

Copy link

@theresa-morrison theresa-morrison left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have reviewed this code by reading it and the associated issue. I think that it accomplishes the goal, but I have not been able to test it. I expect that these changes will not cause problems if you are running MOM5 without the salt flux turned on.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Allow passing salt flux restoring/correction to generic tracers
5 participants