search

ESCOMP / POP2-CESM

Parallel Ocean Program (POP2) in CESM
http://www.cesm.ucar.edu/models/cesm2/ocean/
4 stars 24 forks source link

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

Closed mvertens closed 3 years ago

mvertens commented 3 years ago

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

klindsay28 commented 3 years ago

@mvertens , the comment at https://github.com/ESCOMP/POP2-CESM/blob/2c160ceac42b1450baba132bdabee769526a974a/drivers/nuopc/ocn_import_export.F90#L813 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 commented 3 years ago

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

mvertens commented 3 years ago

@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 commented 3 years ago

@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?

mvertens commented 3 years ago

@alperaltuntas - thank you!!!

On Tue, Jun 15, 2021 at 9:18 AM Alper Altuntas @.***> wrote:

@.**** approved this pull request.

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

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/ESCOMP/POP2-CESM/pull/56#pullrequestreview-684150164, or unsubscribe https://github.com/notifications/unsubscribe-auth/AB4XCE6SJEN7SABBLEFY3ATTS5VLHANCNFSM46KGMCUQ .

-- Mariana Vertenstein CESM Software Engineering Group Head National Center for Atmospheric Research Boulder, Colorado Office 303-497-1349 Email: @.***

alperaltuntas commented 3 years ago

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

mvertens commented 3 years ago

@alperaltuntas - I would add it to cesm2_3_alpha05a.

alperaltuntas commented 3 years ago

done. thanks.