search

NorESMhub / BLOM

Bergen Layered Ocean Model
GNU Lesser General Public License v3.0
16 stars 25 forks source link

remove #ifdefs from HAMOCC #280

Closed mvertens closed 8 months ago

mvertens commented 8 months ago

This PR is a substitute for #264 (which I would like to close). It does not build upon the ability for CMEPS to calculate DMS and BROMO fluxes. The current testing I have done is to compare relative to the latest baseline:

  1. ./create_test --xml-category aux_blom_noresm --xml-machine betzy --xml-compiler intel --gen aux_blom_noresm_13102023 --baseline-root /cluster/shared/noresm/noresm_baselines --compare aux_blom_noresm_27092023 --project nn2345k The following tests were run: ERP_Ld3.T62_tn14.NOINY.betzy_intel - passed ERS_Ld3.T62_tn14.NOINYOC.betzy_intel - passed ERS_Ld3.T62_tn14_wtn14nw.NOINY_WW3.betzy_intel - failed baseline comparison ERS_Ld3.T62_tn14_wtn14nw.NOINY_WW3.betzy_intel.blom-wavice - failed baseline comparison SMS_D_Ld1.T62_tn14.NOINYOC.betzy_intel - passed I think there is something problematic with the current ww3dev code that is leading to failures in the baseline comparison. The same tests without ww3 passed with no problem. I will raise an issue on this - but I don't think this should hold up this PR.
  2. Ran two tests using noresm2.0.6 - SMS.T62_tn14.NOINYOC.betzy_intel. The first used used BLOM master and the second tests used this PR for BLOM. The results were bfb after 5 days.
  3. Ran two fully coupled tests using noresm2.0.6 - SMS.f19_tn14.N1850esm.betzy_intel. The first used used BLOM master and the second tests used this PR for BLOM. The results were bfb after 5 days.
mvertens commented 8 months ago

@JorgSchwinger - I ran a fully coupled test in the noresm2.0.6 code base - as was suggested- and the results were bfb.

jmaerz commented 8 months ago

Out of curiosity: what was/is the purpose of ci.yml that has been removed - is this the setup to do the testing on github? - if so, why was it now removed?

jmaerz commented 8 months ago

@mvertens , @JorgSchwinger , just to let you know, with #282, we updated the beyond-CMIP6 branch with the master - so that this PR can go next into master.

mvertens commented 8 months ago

As an additional test - I compare master versus this PR for a SMS_D_Ld1.T62_tn14.NOINYOC.betzy_intel - but where I had the following xml variables set to TRUE: HAMOCC_CFC: TRUE HAMOCC_CISO: TRUE HAMOCC_NATTRC: TRUE HAMOCC_SEDBYPASS: TRUE And the results were bit-for-bit in comparing the blom history files .hd.0001-01-01.nc .hbgcd.0001-01-01.nc So this gives me more confidence that the tests that @JorgSchwinger was proposing will pass.

mvertens commented 8 months ago

Once I reintroduce ci.yml - the test is now failing. In the past I did notice that periodically it would fail and then pass.

JorgSchwinger commented 8 months ago

@mvertens, @jmaerz, @TomasTorsvik I just tested this PR, and the results for the C-isotopes are NOT bfb!

I'm investigating this further, but this should be clarified before we move on

mvertens commented 8 months ago

@JorgSchwinger - thanks for doing this test. Can you please point me to your rundir and caseroot. I thought I had turned on isotopes last night and it was bfb - but clearly I'm not doing something right.

TomasTorsvik commented 8 months ago

@mvertens , @JorgSchwinger , @jmaerz I will open a new issue on this, with reference to PR #280. We will probably need to do a fix in a new PR anyway, so it's not convenient to keep the discussion in an already merged PR.