Feature iHAMOCC nml #261

jmaerz · 2023-07-04T15:02:35Z

Hi @TomasTorsvik and @JorgSchwinger , I create this draft pull request to follow up on and move the discussion #195 based on the current changes I did for the BGC parameter nml. I restructured the parameter initialization a bit related to the different topics of the parameters.

I currently see a few related questions:

How shall we deal with parameters calculated from other (tunable) parameters? - shall we introduce functions or shall we ONLY calculate them after having read the namelist?
Shall we continue putting atmospheric concentrations (as external controlling parameters) into the former BGC namelist, where atm_co2 is still located? - this would enable to recombine the further calculation of dependent concentrations (isotopes) and initializing the atmosphere fields.
I thus far only introduced TYPICAL tuning parameters in the new nml. I personally would keep it like this or do you have a different opinion?
I would like to delete the line breaks in the write parameter routine - any objections?
I still need to delete the old subroutine that is still present (but not used anymore).

Thus far, I didn't move any of the parameter declarations to get the parameter values under the protect statement. I can do this, if you want me to do it. Just let me know.

…not executable

…ine for parameters that depend on tuning parameters

JorgSchwinger · 2023-08-02T09:21:38Z

hamocc/mo_parambgc_ini.F90

+#endif
+
+  implicit none
+


A thought from my side: Having the variables that qualify as "parameters" defined in the module header of this module would simplify things a lot. Those variables that are truly "parameters" could be defined as such and assigned a value (that isn't changed), e.g.

integer, parameter :: rcar =122

Parameters that can be modified via namelist could be assigned a default value, e.g.

real, save :: atm_co2 = 284.32

Then the problem of having to recalculate things after reading of the namelist would be obsolete, and the module would be much simpler (plus it would be much more transparent to the user - the fact that all parameter definition are scattered between different modules now isn't very nice in my opinion.

Of course this is more invasive and much more work (a lot of use statements would need to be changed).

Hi @JorgSchwinger , seeing the upcoming pull request in #264 (and potentially my nitrogen cycle), I currently would opt for a step-wise approach, so first introducing this less invasive step, and after pulling in Marianas changes doing a second step and following your suggestion, potentially even go the step with the protect statement, what @TomasTorsvik suggested. Let's discuss this in person.

JorgSchwinger · 2023-08-02T09:23:22Z

hamocc/mo_parambgc_ini.F90

+    ENDDO
+    ENDDO
+  end subroutine
+


These 3 subroutines above could be simplified and combined into one routine if we would follow the idea of defining all parameters inside this module (as opposed to importing them)

Hi @JorgSchwinger , I -partially- agree - the calculation and field initialization need to be carried out after reading the namelist, while the initial default initialization could be done before.

I would leave it for now and re-think about it, once introducing the protect statements after #264 was coming in.

JorgSchwinger · 2023-08-02T09:31:19Z

hamocc/mo_parambgc_ini.F90

+    integer  :: iounit
+
+    namelist /bgcparams/ bkphy,dyphy,bluefix,bkzoo,grazra,spemor,gammap,gammaz,ecan,zinges,epsher,bkopal,rcalc,ropal,              &
+                       & remido,drempoc,dremopal,dremn2o,dremsul,fetune,relaxfe,wpoc,wcal,wopal


wpoc would only be used directly if the old HAMOCC sinking (constant wpoc) is used. Shouldn't we rather include wmin/wmax/wlin of the WLIN scheme as tuning parameters? (I would even be tempted to remove the constant sinking completely from HAMOCC, since this produces crap results)

I am totally happy to extend the parameter list of the namelist. So far, I saw the new namelist as extendable and only for relevant parameters.

Done. I left the constant sinking, though - can be discussed afterward in an extra PR.

JorgSchwinger · 2023-08-02T09:33:34Z

hamocc/mo_parambgc_ini.F90

+    !
+    perc_diron = fetune * 0.035 * 0.01 / 55.85
+
+  end subroutine


If this remains the only parameter to adjust, I think it is better to just have this line in the main routine instead of making a separate subroutine.

I am not sure, if I understand, what you consider the main routine here. I guess, my intention was to provide a structure , which one can follow.

TomasTorsvik

Looks fine for me

hamocc/beleg_vars.F90

jmaerz · 2023-08-02T12:13:17Z

To briefly summarize the conversation on this PR: To avoid too many merging conflicts, the potential restructuring wrt protect statements and pulling all parameters into one module is envisaged in an extra PR, once #264 is merged into master.

jmaerz · 2023-08-02T16:41:45Z

Hi @TomasTorsvik and @JorgSchwinger , I tested the branch against the current master using the NOINYOC T62_tn21 setup and ran the model for 1 year and checked successfully for binary identical results.

How to use the new feature (thus far, tested):
After model setup (./case.setup), copy the ./components/blom/cime_config/buildnml to the ./SourceMods/src.blom/ folder (no further subfolders needed). In this copy of buildnml, enter the parameters that you wish to change in the BGCPARAMS namelist (for available parameters, see code, potentially add it, if needed - careful with rates and parameters that are required for further calculation of other parameters - make sure that they are converted and applied after reading the namelist - currently, this should be ensured, but with new developments, this needs care-taking). Parameters available through the user_nl_blom are still possible to change via an additionally provided user namelist for BLOM (so nothing changed in this regards).

I will perform the pull once the boxatm change #265 has been merged into master. Afterward, I will also merge these changes into the feature-HAMOCC_beyond-CMIP6 branch. Once the ifdefs refactoring #264 are merged into master, we can proceed with restructuring the parameter declaration and initialization.

JorgSchwinger · 2023-08-08T13:32:00Z

cime_config/ocn_in.readme

+! NAMELIST BGCPARAMS FOR iHAMOCC-BGC PARAMETERS (DEVELOPERS ONLY)
+! 
+! CONTENTS: EMPTY BY DEFAULT
+!           FOR ADJUSTABLE PARAMETERS, SEE CODE


Once we have settled on a set of parameters, it would be good to list those here

JorgSchwinger · 2023-08-08T14:03:57Z

hamocc/hamocc_init.F90

@@ -48,6 +48,7 @@ subroutine hamocc_init(read_rest,rstfnm_hamocc)
       &                    dtb,dtbgc,io_stdo_bgc,ldtbgc,                       &
       &                    ldtrunbgc,ndtdaybgc,with_dmsph,l_3Dvarsedpor
  use mo_param1_bgc,  only: ks,init_por2octra_mapping
+  use mo_parambgc_ini,only: ini_parambgc


Since we plan to have all parameters defined within the module (in the longer run), the name mo_parambgc_ini is not really the best, in my opinion. mo_param_bgc would be better (but that is too close to mo_param1_bgc?). Not sure what to do about it.

JorgSchwinger · 2023-08-08T14:28:32Z

hamocc/mo_parambgc_ini.F90

+    ! factor for normalizing 14C tracers (~1e-12)
+    c14fac   = atm_c14/atm_co2
+#endif
+  end subroutine 


A matter of personal taste, but I would prefer to have not too many small subroutines. I would place the calculations done here in other routines: The 3 first lines could go into ini_param_atm and the rest into ini_fields_atm (d13C_atm is only used to print out the value, so it could go into write_parambgc)

JorgSchwinger · 2023-08-08T14:39:27Z

hamocc/mo_parambgc_ini.F90

+    call readjust_param()          ! potentially readjust namlist parameter-dependent parameters
+    call rates_2_timestep()        ! Converting rates from /d... to /dtb
+
+    call write_parambgc()          ! write out used parameters and calculate back rates from /dtb to /d..


Wouldn't it make more sense to write out the parameter BEFORE multiplying by dtb? If I want to check the settings of parameters in the log file, I relate much more to the value that I have set via namelist than the value multiplied by dtb.

jmaerz · 2023-08-09T13:16:48Z

To briefly summarize (incl. the short-conversation today), before merging:

After merging, and once remove most hamocc #ifdefs in favorite of new logicals #264 has been merged in, we can i) implement the protect statements and ii) potentially slightly restructure mo_param_bgc wrt to the number of subroutines. This includes potentially shifting the initialization of the atmosphere field into beleg_vars. Further mo_param1_bgc could be renamed to mo_param_tracer in that process.
In write_parambgc, rates are multiplied with an inverse of dtbgc so that the namelist parameter is comparable - not refinding the nml-provided parameter give rise to an intended double-check.
Wrt to explicitly mentioning the BGC nml parameters: we can discuss this again. I would tend to not state them explicitly.

jmaerz added 14 commits July 3, 2023 14:11

renaming beleg_parm to mo_parambgc_ini without any further changes - …

d19cd45

…not executable

adjust description to new module name

b00787c

adjust description

c0dbb6d

initial step to rewrite mo_parambgc_ini to real module - compilable

7850d73

Restructuring mo_parambgc_ini

abecb58

Placing the nml-read call

bda457f

First working namelist read for BGC parameters

0564b0f

Re-arrange parameter initialization and tidy up a bit

66eae1a

add the namelist to the cime config buildnml file - for expert use only

cf420d2

Add missing parameters to the log-writing subroutine

006059f

Declare local parameters for cisonew

d881e30

Remove unused parameter

19860b5

fix compile error wrt AGG=true

6d3b9eb

Add typical tuning parameters to namelist and introduce raedjust rout…

5604fb6

…ine for parameters that depend on tuning parameters

jmaerz added enhancement New feature or request iHAMOCC Issue mainly concerns the iHAMOCC code base labels Jul 4, 2023

jmaerz requested review from JorgSchwinger and TomasTorsvik July 4, 2023 15:02

jmaerz self-assigned this Jul 4, 2023

JorgSchwinger reviewed Aug 2, 2023

View reviewed changes

jmaerz mentioned this pull request Aug 2, 2023

remove most hamocc #ifdefs in favorite of new logicals #264

Closed

TomasTorsvik approved these changes Aug 2, 2023

View reviewed changes

hamocc/beleg_vars.F90 Outdated Show resolved Hide resolved

jmaerz added 2 commits August 2, 2023 13:50

add white space

269f4bb

add Martin curve sinking parameters to bgc-namelist

d1d6fca

jmaerz added 2 commits August 2, 2023 14:27

merged with master

8a76546

Add information to NML-readme file

3a9c37b

jmaerz marked this pull request as ready for review August 2, 2023 16:41

Remove old attached version of beleg_parm

ff59236

jmaerz added 2 commits August 3, 2023 11:15

Cleaning write of parameters

24c794b

add some white space

9d5636d

jmaerz requested a review from JorgSchwinger August 7, 2023 10:16

JorgSchwinger approved these changes Aug 8, 2023

View reviewed changes

jmaerz added 2 commits August 9, 2023 11:35

rename mo_parambgc_ini

ea80d28

Adjust files to new name

a9c367d

jmaerz merged commit 26ea524 into NorESMhub:master Aug 9, 2023

This was referenced Aug 9, 2023

Merge extended nitrogen cycle, M4AGO scheme and preformed Si tracer into master #269

Merged

refactor buildnml to use the CIME-CCS python based namelist generation capabilities #263

Merged

jmaerz deleted the feature-iHAMOC-nml branch November 23, 2023 12:27

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Feature iHAMOCC nml #261

Feature iHAMOCC nml #261

jmaerz commented Jul 4, 2023

JorgSchwinger Aug 2, 2023

jmaerz Aug 2, 2023

JorgSchwinger Aug 2, 2023

jmaerz Aug 2, 2023

jmaerz Aug 2, 2023

JorgSchwinger Aug 2, 2023

jmaerz Aug 2, 2023

jmaerz Aug 2, 2023

JorgSchwinger Aug 2, 2023

jmaerz Aug 2, 2023

TomasTorsvik left a comment

jmaerz commented Aug 2, 2023

jmaerz commented Aug 2, 2023 •

edited

Loading

JorgSchwinger Aug 8, 2023

JorgSchwinger Aug 8, 2023

JorgSchwinger Aug 8, 2023

JorgSchwinger Aug 8, 2023

jmaerz commented Aug 9, 2023

Feature iHAMOCC nml #261

Feature iHAMOCC nml #261

Conversation

jmaerz commented Jul 4, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

TomasTorsvik left a comment

Choose a reason for hiding this comment

jmaerz commented Aug 2, 2023

jmaerz commented Aug 2, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jmaerz commented Aug 9, 2023

jmaerz commented Aug 2, 2023 •

edited

Loading