Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Revise modules for handling of external inputs (N-dep, rinvers, iron)… #168

Merged
merged 2 commits into from
Jun 2, 2022
Merged

Revise modules for handling of external inputs (N-dep, rinvers, iron)… #168

merged 2 commits into from
Jun 2, 2022

Conversation

JorgSchwinger
Copy link
Contributor

…, split into modules for reading the data and modules that apply the fluxes in core hamocc

…, split into modules for reading the data and modules that apply the fluxes in core hamocc
@JorgSchwinger
Copy link
Contributor Author

Following our discussions last week, I wanted to keep the momentum and do the split into reading and applying input-data. In my opinion this is an improvement, as it makes the code structure clearer. I have also moved the definition of the river-indices to mo_param1, such that all indice-definitions are collected there. The "apply"-modules really only encapsulate the corresponding subroutines. We could/should also apply the iron deposition outside ocprod using a dedicated module (as it is now, rivers and n-deposition come after ocprod, while iron is applied at the beginning of ocprod - that doesn't make much sense, but changing this would break bit-for-bit reproducibility). @jmaerz: I still can't add you as reviewer, but maybe you could have a look.

DO i=1,kpie
IF(omask(i,j).GT.0.5) THEN

fdt = dtb/365.
Copy link
Collaborator

@jmaerz jmaerz May 19, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi Jörg, we could do this outside the loop, since it is constant - but we can do this later as well.

+ rivin(i,j,irdip)*fdt/volij
ocetra(i,j,1:kmle,inatalkali) = ocetra(i,j,1:kmle,inatalkali) + rivin(i,j,iralk)*fdt/volij
#endif
ocetra(i,j,1:kmle,iiron) = ocetra(i,j,1:kmle,iiron) + rivin(i,j,iriron)*fdt/volij*0.01
Copy link
Collaborator

@jmaerz jmaerz May 19, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With the fully new restructuring, I feel it would be nice to replace the explicit value of 0.01 by a parameter value (also in line 148), but we can do this later as well.

@jmaerz
Copy link
Collaborator

jmaerz commented May 19, 2022

Hi Jörg, thanks for your initiative! I was going through your changes and left two minor comments that we can also work out later (since they do not necessarily belong to the modularization). With regards to the iron application, I would suggest to open the module files, while (for now?) keeping the order of application (which then, at some point, could be easily changed by re-locating one line) - potentially placing the apply_iron (or similar) already in hamocc4bgm to make ocprod a bit cleaner. That would enable us to still be compatible with the CMIP6 model version, while already having the modular structure in place. What do you think? Wrt to me as reviewer, Tomas sent an email, but it seems that thus far, I haven't been added to NorESMhub.

@JorgSchwinger
Copy link
Contributor Author

Thanks Joeran, all agreed. I have pushed new changes.

@TomasTorsvik TomasTorsvik added the iHAMOCC Issue mainly concerns the iHAMOCC code base label May 20, 2022
Copy link
Contributor

@TomasTorsvik TomasTorsvik left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good initiative, Jörg. As you say, we should probably do the "apply" steps in a more consistent way, but it's good to make the changes in incremental steps (especially if something breaks backward compatibility).

@JorgSchwinger JorgSchwinger merged commit c02fc1a into NorESMhub:master Jun 2, 2022
@JorgSchwinger JorgSchwinger deleted the feature-hamocc_inputdata branch June 2, 2022 09:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
iHAMOCC Issue mainly concerns the iHAMOCC code base
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants