add ability to read input file for ocean alkalinization

[TimotheeBrgs]

Hi all,

Previously, a constant scenario and a linear ramping-up of ocean alkalinization have been implemented in mo_read_oafx.F90, both spatially homogeneous and customizable via the user_nl_blom namelist. Here, a third case is added for scenarios defined from a file oalkfile prescribing monthly 2D fields of surface alkalinity addition in kmol ALK m-2 yr-1.

Now, to use the constant or ramping-up scenario, the user needs to define only the variable oalkscen and leaves oalkfile undefined (default value is ''). To use the scenario from oalkfile, the user needs to define only oalkfile and leaves oalkscen undefined (default value '' as well). Defining both oalkscen and oalkfile returns an explicit error.

Because this case defined from a file is the same procedure as the N deposition, this commit is mostly coming from a copy-paste of mo_read_ndep.F90 blocks.

Example of user_nl_blom to use this case:

BGCOAFX_DO_OALK = .true.
BGCOAFX_OALKFILE = '/path/to/file.nc'

[TomasTorsvik]

I wonder if it would simplify the implementation a bit if you introduced reading from a file as a separate scenario, e.g. oalkscen=='file', so you don't need to test for both oalkscen and oalkfile in the if statements.

Otherwise I think the PR look fine.

[JorgSchwinger]

I agree with Tomas here. I would think setting oalkscen=='file' to indicate that the scenario is defined by an inputfile (which then requires the specification of oalkfile) is preferable. It simplifies the coding, but it is also less prone to user errors since there is no requirement that oalkfile needs to be empty in the case of the const scenarios.

[TimotheeBrgs]

Hi Thomas and Jörg, thank you for your review. I submitted another commit following your suggestion of having to define both oalkscen='file' and oalkfile='path/to/file.nc' to use the scenario based on a file.

[TomasTorsvik]

Thanks @TimotheeBrgs !

[JorgSchwinger]

Since there is a section "Local variables" just below, it would be nice to move current_day there.

[JorgSchwinger]

Looks very good. Once you have tested that everything work, let me know, then I'll merge it.

[TomasTorsvik]

Hi @JorgSchwinger @TimotheeBrgs ,
I did a test today, and realized that BLOM stops at runtime if there is no &BGCOAFX entry in the namelist file. It would be better if we could have this namelist as optional.

I therefore suggest that we revert back to reading DO_OALK in the &BGCNML entry in the namelist, and use this to determine if BLOM needs to read &BGCOAFX. Would this be OK for you?

If so, can this change be included in the current PR? Otherwise, I can open a new one.

[JorgSchwinger]

Yes that sound good. This also would also be consistent with the fact that do_oalk is defined in mo_control_bgc, whereas the rest of oalk-related variables is defined in mo_read_oafx and can be read there only if do_oalk=true.

How to do this technically? Should Timothée incorporate this into this PR, or should it be done separately?

[JorgSchwinger]

It is probably easier to do this in the current PR, otherwise the current PR would run into merge conflicts.

[jmaerz]

Just as a side note - the file can be empty. It just needs to be there.

[TomasTorsvik]

Great!
@TimotheeBrgs
Could you also look into hamocc_init : remove oalkfile and oalkscen from use statement (L59) and from namelist definition (L83), so that we don't run the risk of reading these twice?

[TimotheeBrgs]

Hi again, thank you all for the bug discovery and improvement suggestions. I hope that the commits 5d37e37 and 2ae28c8 will solve them.

@TomasTorsvik DO_ALK is now defined back in the BGCNML section of buildnml. For consistency, I renamed back the namelist shell variable DO_OALK instead of BGCOAFX_DO_OALK (previous renaming done in PR 224). The value of do_oalk is no longer read from the namelist in mo_read_oafx.F90, as it is defined in mo_control_bgc. The reading of oalkfile and oalkscen in hamocc_init is removed.

@JorgSchwinger current_day is now correctly declared in the "local variable" section of the get_oafx subroutine.

@jmaerz I am not sure if I understand the potential issue raised by your message. Do you suggest to include the triggering of an error if the netCDF file is empty? Or to trigger an error telling "oalkfile is undefined" if oalkscen='file' and oalkfile=''?

Now, with the current commits, the way to use the file scenario is the following:

DO_OALK = .true.
BGCOAFX_OALKSCEN = 'file'
BGCOAFX_OALKFILE = '/path/to/file.nc'

[jmaerz]

Hi @TimotheeBrgs , sorry for the confusion. It's all fine from my side - I just wanted to note that one could live with an empty BGCOAFX nml file. But the solution now looks way better.

[TomasTorsvik]

I think in hamocc_init, you still need to remove oalkfile and oalkscen in the namelist definition (L83).

[TimotheeBrgs]

Hi @TomasTorsvik , good point! With the last two commits, I removed what you suggest and I added a few missing explanatory comments in buildnml. It compiles correctly and run as it should, with or without do_alk defined.