remove #ifdefs from HAMOCC #280
Conversation
…mediator OR send the fluxes back
…assed back from mediator
hamocc/mo_param_bgc.F90
Outdated
| #ifdef cisonew | ||
| REAL :: beta13, alpha14, d14cat, d13c_atm | ||
| #endif | ||
| REAL :: dustd1, dustd2, dustsink !AGG |
There was a problem hiding this comment.
I am running into compilation problems - the line with dustd1, dustd2, dustsink needs to be deleted. (They are introduced now via mo_biomod - it was formerly a bit twisted written.)
There was a problem hiding this comment.
I don't understand this comment. I have no problems compiling.
There was a problem hiding this comment.
Hi @mvertens , dustd1, dustd2, dustsink are already included by use mo_biomod, only: above so that at least on my side, the compiler complains Symbol ‘dustd1’ at (1) conflicts with symbol from module ‘mo_biomod’, use-associated at (2) (compiled locally via meson). Similarly, dustd2 and dustsink are also already defined. As written, it was very twisted before (these were only included via use when AGG was switched on, while then the local variables were required inside the routine for some initialization). Deletion of the line
REAL :: dustd1, dustd2, dustsink !AGG
should do the job.
There was a problem hiding this comment.
Yes - you are right. Sorry for not seeing that.
There was a problem hiding this comment.
No worries - I am happy to be able to help :-)
There was a problem hiding this comment.
I've pushed the fix back.
|
Out of curiosity: what was/is the purpose of |
.github/workflows/ci.yml
Outdated
There was a problem hiding this comment.
This file is removed in commit 5957d2c
What was the background for this? If it is not working, is there some way to disable CI functionality instead of removing it completely?
| #ifdef HAMOCC | ||
| logical :: use_hamocc = .true. | ||
| #else | ||
| logical :: use_hamocc = .false. |
There was a problem hiding this comment.
Would it make sense to move this to mod_ifdefs.F90, and make use of use_hamoccin both restart_trcrd.F90 and restart_wt.F? Not sure if this is a good idea, if the long term plan is to get rid of mod_ifdefs.F90.
|
@mvertens , @JorgSchwinger , just to let you know, with #282, we updated the |
|
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: |
|
Once I reintroduce ci.yml - the test is now failing. In the past I did notice that periodically it would fail and then pass. |
| if (use_HAMOCC) then | ||
| call hamocc_init(1,rstfnm_hamocc) | ||
| end if |
There was a problem hiding this comment.
| if (use_HAMOCC) then | |
| call hamocc_init(1,rstfnm_hamocc) | |
| end if | |
| #ifdef HAMOCC | |
| call hamocc_init(1,rstfnm_hamocc) | |
| #endif |
The CI intel compiler build fails on the call to hamocc_init. When meson builds without the 'ecosys' option it does not compile code in the hamocc directory, so then hamocc_init is not available.
It may be safer for now to keep the calls to hamocc funtions/subroutines in #ifdef, when called from outside hamocc.
There was a problem hiding this comment.
I tried to update the ci.yml file to include tests with 'ecosys' enabled, but the tests are still failing. Not sure what's going on, but the meson build system doesn't seem to recognize ecosys=true.
There was a problem hiding this comment.
@TomasTorsvik - thanks for looking into this. I agree that it makes sense at this point to keep the #ifdef HAMOCC on.
Are we okay with merging this PR - or do we want to resolve the meson build system problem first?
There was a problem hiding this comment.
We can merge this now. I can add the meson build afterwards, I didn't enable it properly with ecosys==true.
TomasTorsvik
added
enhancement
|
@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 |
|
@JorgSchwinger - thanks for doing this test. Can you please point me to your rundir and caseroot. |
|
@mvertens , @JorgSchwinger , @jmaerz |
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:
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.