Read BGC input files through pkg/gchem #876
Conversation
|
Hi @averdy
I'll have a close look, once this is done, but for naming I suggest that the field names need not necessarily include "exf", because one can image reading the same fields via the "old" method. For a cleaner separation, we could think of having a separate routine |
|
@averdy Thanks for starting this. I suggest to wait a little bit before any new updates here to give us some time to provide feedback. |
|
I needed to have a data with commented out, but with these changes: I can now run input.exf, but the default fwd experiment fails. |
|
@mjlosch I'm having computing issues and can't run the model right now, but I'll look into it as soon as possible! I'm sharing this as a draft mostly to get feedback on the general structure. I'm not attached to any of it at this point! |
avoids using unitialised filenames that make reference experiment global_oce_biogeo_bling fail (and it is better style)
There was a problem hiding this comment.
I think that this is great. I have a few inline comments and suggestions.
I would even go a step further and make pkg/bling (and pkg/dic in a similar way) entirely agnostic of where the forcing fields come from. A gchem_fields_load loads all all forcing data provided by data.gchem into fields called gchem_wspeed, etc, and it is here, where we decide if we want to use the ext-method or the original simple method. bling_fields_load would then just copy to forcing fields local to bling (or dic), or uses the gchem-fields explicitly (i.e. bling_fields_load would no longer be necessary). Each BGC package called from gchem can use the fields or decide not to use them and have their own method of loading them.
| STOP 'ABNORMAL END: S/R INSOL: 2-D output not implemented' | ||
| ENDIF | ||
|
|
||
| C Find day (****NOTE for year starting in winter*****) |
There was a problem hiding this comment.
This goes a little further, but this bit is basically a copy of dic/insol.F, isn't it? Would it make sense to have a routine pkg/gchem/gchem_insol.F (or a similar name) that then handles the computation of irr_surf if irr_surf is not read from file?
Or even have this routine pkg/gchem/gchem_insol.F return some irr_surf that is either computed from these astronomical parameters or retrieved from a file, either by exf or not, it could even include the QSW_underice part.
There was a problem hiding this comment.
@mjlosch Yes, it's a copy of dic/insol.F and I agree that it would make sense to have a common routine
I have a version in my local repo, where I have done this. It gives exactly the same solution (via testreport) as the current code. It's not complete (e.g. some of the runtime parameters such as |
This reverts commit eb83575.
- bling_fields_load.F only contains a copy and possibly a unit conversion - replace exf_ prefix by gchem_ (not fully consistent yet) - always call gchem_exf_readparams - gchem_fields_load.F contains all reading of forcing files, with and without exf; logic may not yet be optimal when to read with pkg/exf - some sensible initialisation of fields in gchem_init_fixed.F is necessary to main results of reference experiment - adjust namelists of reference experiment(s) - add preliminary reference output for testing - this does not yet work with TAF AD because variable names have changed and need adjustment; postponed to when naming changes are complete
tutorial_cfc_example and tutorial_global_oce_biogeo now compile and run
|
I did not pay to much attention to other verification experiments than I note that none of this works with TAF, because variable names have changed and variable have moved. We will sort this out later, when we have converged on a complete set of names of the new gchem forcing variables. A few thoughts:
|
- rename forcing fields to have prefix "gchem" (for now without _) - forcing file names in general gchem namelist, maybe it is even better to move all of gchem_exf_readparms.F to gchem_readparms.F, including the forcing namelist? - foward testreport passes (at least on my computer)
|
@averdy the latest commit breaks some of the verification experiments with |
|
@mjlosch Fixed, thanks! |
|
The largest changes are from a63c177 where initial conditions are changed. I think we should stick to the old initial conditions to be able to reproduce the old results for now, so that we don't get confused. In a separate PR we can still change these defaults if really necessary. |
It has to do with the initialisation of |
|
Not sure I understand why gchem_pco2 is needed. The exf package already has a mechanism for setting a constant value. Why not use apCO2const? If I understand the proposed changes, this is a runtime parameter but ignored. This is kind of unexpected. Same for the hard-coded default values for the other fields set in gchem_init_varia.F. Apologies if I am missing something, just trying to catch up. |
|
There was I totally see that this is redundant, and we can use The hard coded default values are only here to be able to reproduce old results (which already does not work anymore). This is now modelled a bit after the bling and dic-verification experiments, because that's what we have. There is no problem in modifying the defaults as soon as there's a consensus what they should be (probably not zero in some cases). |
What changes does this PR introduce?
Feature: BGC input files are handled through pkg/gchem. A more general approach than what was initially proposed for pkg/bling ( PR #841 ), as it can be used for other BGC models. Addresses issue #138
What is the current behaviour?
BGC forcing (surface iron flux, atmospheric pCO2, wind speed, atmospheric pressure, ice) and fields that are not prognostic variables (surface silica) are specified in namelist files data.dic, data.bling, or data.darwin and read in the package-specific *_fields_load.F routine.
What is the new behaviour
Those fields are now read in gchem_fields_load.F and handled through data.gchem. It is done using pkg/exf routines if useEXF (or maybe useGCHEM_EXF?) is true; otherwise it is done using either the current method (READ_REC_XY_RS).
Does this PR introduce a breaking change?
For pkg/bling, the only change required is switching BGC forcing fields info to data.gchem. For other packages, no breaking change - by default, forcing fields read with gchem will not be passed on to the package.
Other information:
Suggested addition to
tag-index