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

In generic_tracer_source call, perform unit conversion for internal heat only when it is applicable #688

Conversation

yichengt900
Copy link

@yichengt900 yichengt900 commented Jul 19, 2024

This PR is related to another PR made in the NOAA-GFDL/ocean_BGC repo.

In the past, setting DO_GEOTHERMAL = True was necessary for COBALT to function correctly. Failure to enable geothermal heating will lead to segmentation faults if someone turned it off accidently.

We have now made internal_heat an optional argument in the generic_COBALT_update_from_source. The if(present()) check will determine if internal_heating is a null pointer. However, in the unit conversion case, the scaling operation G%US%RZ_to_kg_m2*US%C_to_degC*tv%internal_heat(:,:) occurs before passing the data to generic_tracer_source. This will cause a segmentation fault if internal_heat is null.

This PR (proposed by @andrew-c-ross originally) fixes this issue by omitting this argument if internal_heat is not associated.

Copy link
Member

@Hallberg-NOAA Hallberg-NOAA left a comment

Choose a reason for hiding this comment

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

I agree that these changes correct the problem that is being described.

In the longer, term, it might be even better if we could arrange to (optionally) pass a MOM unit_scale_type argument into generic_tracer_sources(). This would have the benefits of limiting the code-duplication in MOM_generic_tracer.F90 while also allowing us to propagate the dimensional consistency testing for physical variables into the generic tracer code.

@adcroft
Copy link
Member

adcroft commented Jul 26, 2024

@yichengt900 Can you check "Allow edits and access to secrets by maintainers" at the bottom of the right hand column (also when you submit a PR)? Our branch rules block merges for out of date code w.r.t. dev/gfdl. That option allows us to update the branch.

@yichengt900
Copy link
Author

Thanks, @adcroft. I think Marshall pointed it out the same issue, and it appears that the option Allow edits and access to secrets by maintainers does not show up when merging an organization fork (weird). I have manually merged the recent updates into my branch.

@adcroft
Copy link
Member

adcroft commented Jul 26, 2024

@adcroft adcroft merged commit 0440ed4 into NOAA-GFDL:dev/gfdl Jul 26, 2024
10 checks passed
@Hallberg-NOAA Hallberg-NOAA added the bug Something isn't working label Nov 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants