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

cam_noresm2_1_v1.0.2: Fix ice delimiter bug fix #125

Merged

Conversation

gold2718
Copy link

@gold2718 gold2718 commented Jan 11, 2024

Summary: Add a compile-time option to revert the ice delimiter to the NorESM2.0 behavior

Contributors: gold2718

Reviewers: oyvindseland, DirkOlivie

Purpose of changes: Allow revert ice delimiter fix (#124)
Also, fix some usermods directories to turn on AEROCOM output as described in the user manual.

Github PR URL: #125

Changes made to build system: None

Changes made to the namelist: None

Changes to the defaults for the boundary datasets: None

Substantial timing or memory changes: None

Added a compile time switch (NORESM2_ICE_DELIMITER) that reverts the ice delimiter behavior to match NorESM2.0.

  • aux_cam_noresm:
    • BASELINE failures for NF compsets (expected due to bug fixes and ice delimiter change)
    • NLCOMP failure (namelist change) for F2000climo compset due to change in land ice from CISM2%NOEVOLV to SGLC.
  • Ran test ERP_D_Ln9_P18.f19_f19.NF1850.betzy_intel.cam-outfrq9s three ways:
    1. "Default" NorESM2.1 configuration (with ice delimiter fix), compared with version before NorESM2 option (PASS)
    2. NorESM2.1 configuration modified to use NorESM2.0 ice delimiter (by defining NORESM2_ICE_DELIMITER), compared with NorESM2 code version (substituted NorESM2 version of src/NorESM/micro_mg2_0.F90 -- PASS)
    3. "Default" NorESM2.1 configuration (with ice delimiter fix), compared with NorESM2 code version (BASELINE FAIL as expected)

resolves #124

@gold2718 gold2718 added the bug Something isn't working label Jan 11, 2024
@gold2718 gold2718 added this to the NorESM2.1 milestone Jan 11, 2024
@gold2718 gold2718 self-assigned this Jan 11, 2024
@DirkOlivie
Copy link

Hi, I would make the comment a little bit more explicit. Now it says "Uncomment the line below to recover the NorESM2 behavior". Maybe write "Uncomment the line below to recover the NorESM2 behavior for micro_mg2_0.F90" (as the other 3 bug-corrections will remain active).

Looks fine for the rest.

@gold2718
Copy link
Author

Hi, I would make the comment a little bit more explicit. Now it says "Uncomment the line below to recover the NorESM2 behavior". Maybe write "Uncomment the line below to recover the NorESM2 behavior for micro_mg2_0.F90" (as the other 3 bug-corrections will remain active).

Good point. How about "Uncomment the line below to recover the NorESM2 ice delimiter behavior."?

I could also include information from the release notes which currently reads:

Removed an artificial limit on the number of ice particles.
See #110

Thoughts?

@DirkOlivie
Copy link

Hi, the text "Uncomment the line below to recover the NorESM2 ice delimiter behavior" is fine for me.

@gold2718
Copy link
Author

Hi, the text "Uncomment the line below to recover the NorESM2 ice delimiter behavior" is fine for me.

@DirkOlivie, If it is okay now, could you 'finish' the review and approve it?

@gold2718
Copy link
Author

@oyvindseland, do you want to review this PR?

@gold2718 gold2718 removed the request for review from oyvindseland January 18, 2024 09:10
@DirkOlivie
Copy link

Yes, fine for me.

Copy link

@DirkOlivie DirkOlivie left a comment

Choose a reason for hiding this comment

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

Ok for me.

@oyvindseland
Copy link

oyvindseland commented Jan 18, 2024 via email

@gold2718 gold2718 merged commit 4451aa4 into NorESMhub:noresm2_1_develop Jan 18, 2024
@gold2718 gold2718 deleted the fix_ice_delimiter_bug_fix branch January 18, 2024 13:04
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.

None yet

3 participants