Fix sediment diagnostics and sediment quality #511

jmaerz · 2025-03-10T15:48:28Z

Hi @TomasTorsvik and @JorgSchwinger ,
I realized the issue, why diagnosed burial fluxes (and some extended nitrogen cycle diagnostics) were overestimated/not correctly estimated - now setting those at each time step to zero, before performing calculations (i.e. burial fluxes are only calculated once per day and not per time step, which overestimated the burial fluxes by factor 24 - similarly, rates for processes that only happen below certain thresholds, etc.). In that process, I also updated the sediment quality calculations to account for the sediment acceleration scheme and in general fixed the burial POC age (previously underestimated).

I set up a run - and at least the diagnostics now look reasonable. Wrt sediment quality, it remains still a bit experimental, while promising - we can discuss details in person, if needed.

TomasTorsvik

@jmaerz - thanks for fix, looks fine to me!

JorgSchwinger · 2025-03-11T10:43:25Z

hamocc/mo_sedshi.F90


    sedfluxb(:,:,:) = 0.

+    if(do_sedspinup .and. kplyear>=sedspin_yr_s .and. kplyear<=sedspin_yr_e) then
+      ! accumulated time spent due to sediment acceleration
+      acc_time = 86400.*sedspin_ncyc/31104000. ! *dtbgc/dtbgc


What is the number 31104000? A comment explaining this would be nice.

Also, this depends on the fact that sedshi is called once per model day, right? Maybe it would be good to add a short comment in hamocc4bcm line 364 that there is a dependency on this choice in the sediment code.

Hi @JorgSchwinger , maybe the number is incorrect? - it's a model year with 360 days in seconds - but maybe better refer to 365 days? (How is it handled in BLOM/NorESM? - naive question... - shall we maybe better introduce a parameter for this in mo_param_bgc?). Yes, it's due to the fact that sedshi is only called once per day, as far as I understand from hamocc4bcm.

If the number is meant to represent seconds in a model year, then it should be 86400*365. Is this number used more often in the code? Then it might really be worth to replace it by a parameter in mo_param_bgc

It could be useful to include parameters for both

seconds_in_day = 86400. seconds_in_year = 86400.*365.

However, I see also references to different calendars in phy/mod_calendar.F90, so I'm not sure if seconds_in_year should be a fixed parameter.

The calendar is defined in phy/mod_time.F90 : init_timevars. The cesm case is run with noleap = 365.
Other cases use different calendars.

I don't think we will use something different than a 365 day calendar (at least not in the near future). One could hard code it in HAMOCC, query the BLOM calendar in hamocc_init and stop if the calendar type is not 365.

The fuk95 and single_column test cases use 360_day calendar, so these will not work if the 365 day calendar is hard coded. We usually run these only for short tests, so maybe a warning is sufficient.

I suppose some of the input files assume a 365 day calendar. Not sure what is the best approach here, but I can see that we can run into trouble if we allow different calendar options.

I propose to make a new issue / PR regarding calendar setting in iHAMOCC, not to bundle everything into this PR.

Thus far no rigorous changes throughout the code - just for sediment quality

jmaerz · 2025-03-11T13:14:34Z

Hi @TomasTorsvik and @JorgSchwinger , I just included some routine to handle the question. The only calendar that I feel issues with is the standard calendar, which seem to handle leap versus non-leap years - I wonder, if we need to go to this detail right now or if it is ok to stop the simulation (or set the days to 365 or 365.25). Opinions?

In any case, I see that some parameter settings rely on sec_per_day and on sec_per_year - for the latter, I don't know, if adjustments are really needed - I expect not. However, for e.g. nitrogen deposition, etc. the adjusted values for the year could be used as well (looks like only a few occasions, where 86400 and 365 appear in the code, which could now be easily exchanged).

TomasTorsvik · 2025-03-11T13:21:45Z

hamocc/mo_param_bgc.F90

+      ! see mod_calendar.F90 and mod_time.F90
+      case ('noleap','365_day')
+        days_per_year = 365.
+      case ('all_leap','366_day')


Suggested change

case ('all_leap','366_day')

case ('all_leap','366_day')

if (mnproc == 1) then

write (io_stdo_bgc,*) 'WARNING: Init iHAMOCC time variables: non-standard 366_day calendar selected'

endif

I suggest to include a warning message if anythin other than 'noleap' is selected.

I'll do so. Sorry, I overlooked your previous comment on doing all this in a different PR (was already working on it...) - shall I try to split it up or is it ok to proceed?

Since you are already working on it, it is probably best to proceed, unless the rabbit hole goes too deep ...

JorgSchwinger · 2025-03-11T13:33:51Z

hamocc/mo_param_bgc.F90

+  !********************************************************************
+  real, parameter :: sec_per_day = 86400. ! seconds per day
+  real, protected :: sec_per_year         ! seconds per year
+  real, protected :: days_per_year        ! simulated days per year


Can we set default values also for sec_per_year and days_per_year, such that something is used even if ini_bgctimes is not called?

Propose some - based on a 365 year?

Thus far, we're rather free to start off with some idea, how to best handle it in the future. In any case, ini_bgctimes is called early in in the initialization by purpose...

I.e. I only see very minor issues in the parameter definition - where a few 365 are hard-coded (while I am not seeing a need to change this).

Propose some - based on a 365 year?

Yes, that would make sense.

jmaerz · 2025-03-11T14:16:46Z

Hi @JorgSchwinger and @TomasTorsvik , I would start a short testrun with the current branch. If that results in reasonable results, I would merge it as is (thus far only tested in the single column mode).

Afterward, we can discuss, in how far we should exchange the 365 in the N-deposition, oafx and river-applications.

matsbn · 2025-03-12T14:29:40Z

Note that nday_in_year can change during a simulation (not the case yet in NorESM), so maybe it should not only be set during initialization?

jmaerz · 2025-03-12T14:33:46Z

@matsbn , what do you mean by "it can change" during a simulation? - with the latest change, I only make use of it once at initialization (when calling hamocc_init) to set some values in iHAMOCC. Would that somehow be affected by what you say?

jmaerz · 2025-03-12T14:42:29Z

Ok, you're probably referring to restarts and leap years, etc. - which would then necessitate to update the value throughout run time?

JorgSchwinger · 2025-03-12T15:25:05Z

Not only restarts, but if it is a Gregorian calendar, nday_per_year changes every 4th year.

I don't know if there are plans to support such calendar with NorESM anytime soon. So for now maybe just issue an error message and stop if calendar is set to Gregorian?

jmaerz · 2025-03-12T16:05:01Z

Hi @JorgSchwinger , while I see an issue for the initial parameter settings (where I wouldn't change the parameter according to year length), I don't see good reasons, why not updating the time accordingly. I now update the time also at runtime (while admittedly the warning is currently not taking always effect in case of non- versus leap years - up to potential change).
In terms of days_per_year: for the current purpose (that might also be up to change), it doesn't make a huge difference. For the deposition reading, etc. we still need to discuss, how to treat it (beyond the scope of this PR - debating currently to branch that time issue completely out, since I would like to have the fix for diagnostics in soon). So technically, we could also hard-set it to 365 at the moment and then debate, if we really need to take different calendars into account.

jmaerz · 2025-03-12T17:17:38Z

Hi @JorgSchwinger and @TomasTorsvik ,
since the time-issue developed into a longer discussion on the purpose, I now reverted back some stuff and will open a new PR on this question to address #512 . I merge now these changes that address the header of this PR and will link to this discussion when opening the new PR (after merging this one).

jmaerz added 3 commits March 10, 2025 15:36

fix sediment diag

df46a95

sedshi - sediment quality fixes

918f839

account for sediment acceleration for burial age

907ade3

jmaerz added the iHAMOCC Issue mainly concerns the iHAMOCC code base label Mar 10, 2025

jmaerz added this to the BLOM v1.8.0 tag milestone Mar 10, 2025

jmaerz self-assigned this Mar 10, 2025

TomasTorsvik approved these changes Mar 11, 2025

View reviewed changes

jmaerz marked this pull request as ready for review March 11, 2025 10:09

jmaerz requested a review from JorgSchwinger as a code owner March 11, 2025 10:09

JorgSchwinger approved these changes Mar 11, 2025

View reviewed changes

jmaerz mentioned this pull request Mar 11, 2025

Set consistent calendar option in iHAMOCC #512

Open

TomasTorsvik linked an issue Mar 11, 2025 that may be closed by this pull request

Set consistent calendar option in iHAMOCC #512

Open

Introducing calendar-based time variables in iHAMOCC

c999921

Thus far no rigorous changes throughout the code - just for sediment quality

TomasTorsvik reviewed Mar 11, 2025

View reviewed changes

TomasTorsvik approved these changes Mar 11, 2025

View reviewed changes

add warning message for non-standard calendar types

b4c172e

JorgSchwinger reviewed Mar 11, 2025

View reviewed changes

jmaerz added 2 commits March 11, 2025 14:56

set defaults before updating

9acc929

replace 86400 by parameter

081c8f5

jmaerz added 2 commits March 11, 2025 16:59

minor modification for restart

757a94b

use "nday_in_year" instead of calendar cases

54ecf9b

update days per year also at runtime

5082755

Reverted back to stage before introducing the calendar-dependency

2568834

jmaerz added 3 commits March 12, 2025 18:05

change hard-coded year length to 365 days

477d7c1

fix restart messages

29c1a02

add note on sedshi-dep on daily update

e62e143

jmaerz merged commit 416fb74 into NorESMhub:master Mar 12, 2025
4 checks passed

jmaerz mentioned this pull request Mar 12, 2025

Calendar dependency of HAMOCC processes #514

Open

jmaerz deleted the fix_sed_diag branch March 12, 2025 17:47

TomasTorsvik mentioned this pull request Mar 18, 2025

NorESM3_0_alpha01 NorESMhub/NorESM#651

Closed

12 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix sediment diagnostics and sediment quality #511

Fix sediment diagnostics and sediment quality #511

jmaerz commented Mar 10, 2025 •

edited

Loading

TomasTorsvik left a comment

JorgSchwinger Mar 11, 2025

JorgSchwinger Mar 11, 2025

jmaerz Mar 11, 2025

JorgSchwinger Mar 11, 2025

TomasTorsvik Mar 11, 2025

TomasTorsvik Mar 11, 2025

JorgSchwinger Mar 11, 2025

TomasTorsvik Mar 11, 2025

TomasTorsvik Mar 11, 2025

jmaerz commented Mar 11, 2025 •

edited

Loading

TomasTorsvik Mar 11, 2025

TomasTorsvik Mar 11, 2025

jmaerz Mar 11, 2025

TomasTorsvik Mar 11, 2025

JorgSchwinger Mar 11, 2025

jmaerz Mar 11, 2025

jmaerz Mar 11, 2025

jmaerz Mar 11, 2025

JorgSchwinger Mar 11, 2025

jmaerz commented Mar 11, 2025

matsbn commented Mar 12, 2025

jmaerz commented Mar 12, 2025

jmaerz commented Mar 12, 2025

JorgSchwinger commented Mar 12, 2025

jmaerz commented Mar 12, 2025

jmaerz commented Mar 12, 2025

Fix sediment diagnostics and sediment quality #511

Fix sediment diagnostics and sediment quality #511

Conversation

jmaerz commented Mar 10, 2025 • edited Loading

TomasTorsvik left a 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

Choose a reason for hiding this comment

jmaerz commented Mar 11, 2025 • 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

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

jmaerz commented Mar 11, 2025

matsbn commented Mar 12, 2025

jmaerz commented Mar 12, 2025

jmaerz commented Mar 12, 2025

JorgSchwinger commented Mar 12, 2025

jmaerz commented Mar 12, 2025

jmaerz commented Mar 12, 2025

jmaerz commented Mar 10, 2025 •

edited

Loading

jmaerz commented Mar 11, 2025 •

edited

Loading