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

Additional bug fixes for ndep and co2 from cam -> pop for nuopc cap #56

Merged
merged 4 commits into from
Jun 15, 2021

Conversation

mvertens
Copy link
Collaborator

@mvertens mvertens commented Jun 8, 2021

Description of changes:

Additional bug fixes for ndep and co2 from cam -> pop for nuopc cap only

Testing:

SMS_Vmct_Ln6.f19_g17.1850_CAM60_CLM50%BGC-CROP_CICE_POP2%ECO_MOSART_SGLC_WW3_BGC%BPRP.cheyenne_intel
SMS_Vnuopc_Ln6.f19_g17.1850_CAM60_CLM50%BGC-CROP_CICE_POP2%ECO_MOSART_SGLC_WW3_BGC%BPRP.cheyenne_intel
SMS_Vmct_Ld5.f19_g17.1850_CAM60%WCTS_CLM50%BGC-CROP_CICE_POP2%ECO%NDEP_MOSART_SGLC_WW3.cheyenne_intel.pop-default
SMS_Vnuopc_Ld5.f19_g17.1850_CAM60%WCTS_CLM50%BGC-CROP_CICE_POP2%ECO%NDEP_MOSART_SGLC_WW3.cheyenne_intel.pop-default

Test case/suite: Just ran above tests

Test status: Should not effect any other tests

Fixes:

User interface (namelist or namelist defaults) changes? None

@mvertens mvertens marked this pull request as draft June 8, 2021 15:27
@mvertens mvertens added the bug Something isn't working label Jun 8, 2021
@klindsay28
Copy link
Collaborator

@mvertens , the comment at

do ncol = 2,mcog_ncols ! same as ice_ncat

needs improvement, as mcog_ncols is equal to ice_ncat+1 (if mcog is enabled).
I think an improved comment is

! mcog_ncols is the same as ice_ncat+1

Do you feel comfortable changing that in this PR? (Its only connection to PR's content is that it is NUOPC related.)

@mvertens mvertens requested a review from klindsay28 June 8, 2021 15:48
@mvertens
Copy link
Collaborator Author

mvertens commented Jun 8, 2021

@klindsay28 - thanks for pointing this out. Comment has been fixed and pushed back.

@mvertens mvertens marked this pull request as ready for review June 14, 2021 22:28
@mvertens
Copy link
Collaborator Author

@alper, @klindsay - I think this is now ready for review. I've also issued a PR for cmeps for the Faoo_fco2_ocn issue.

@mvertens
Copy link
Collaborator Author

@alperaltuntas - this PR is needed for the next prealpha tag to fix several problems found in the testing and validation. Would it be possible prioritize getting it in?

Copy link
Member

@alperaltuntas alperaltuntas left a comment

Choose a reason for hiding this comment

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

looks good to me. @mvertens, Unless I hear a change request from @klindsay28, I will merge this soon.

@mvertens
Copy link
Collaborator Author

mvertens commented Jun 15, 2021 via email

Copy link
Collaborator

@klindsay28 klindsay28 left a comment

Choose a reason for hiding this comment

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

Looks good to me. Thanks @mvertens for this.

@alperaltuntas alperaltuntas merged commit cf17aeb into ESCOMP:master Jun 15, 2021
@alperaltuntas
Copy link
Member

@mvertens , Should this be added to cesm2_3_alpha05a or cesm2_3_alpha05b?

@mvertens
Copy link
Collaborator Author

@alperaltuntas - I would add it to cesm2_3_alpha05a.

@alperaltuntas
Copy link
Member

done. thanks.

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