E3sm: I1850CNPECACNTBC run with `debug=.true.` crashes

[bishtgautam]

@jinyuntang found that I1850CNPECACNTBC run with debug=.true. crashes because dyn_nbal_adjustments_col and dyn_pbal_adjustments_col are not set to zero. The fix includes uncommenting the following lines :

• PhosphorusStateType.F90#L2270
• CNNitrogenStateType.F90#L2261

[jinyuntang]

@bishtgautam, related to this, I further found that CNNitrogenFluxType.F90#L2602 should be lifted into the do loop right above to avoid another floating error.
Similarly for
PhosphorusFluxType.F90#L2078, which should be lifted into the do loop right above.

[bishtgautam]

@jinyuntang, To make sure I understand you correctly. The this%livestemn_to_litter_patch and this%livestemp_to_litter_patch shouldn't be within the if (crop_prog)-loop. Correct?

[jinyuntang]

Yes.

Sent from my iPhone

On Feb 9, 2019, at 11:12 AM, Gautam Bisht [e-mail] wrote:

@jinyuntang, To make sure I understand you correctly. The this%livestemn_to_litter_patch and this%livestemp_to_litter_patch shouldn't be within the if (crop_prog)-loop. Correct?

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub, or mute the thread.

[minxu74]

Do we need the variables of livestemn_to_litter_patch and livestemp_to_litter_patch? They are just initialized to NaN and used directly to calculate other variables. I did not see any definitions for them with physical meanings in the code.

[thorntonpe]

@minxu74 The variables livestemn_to_litter and livestemp_to_litter are valid variables and need to be retained. They are assigned meaningful values in the phenology routines.

[minxu74]

@thorntonpe Thanks a lot for the information. I see the assignments. If I understand them correctly, the variables are assigned only when ivt(p) >= npcropmin? If so, they should be with in the crop_prog if statement. But we should not use it when crop_prog=.False.?

[thorntonpe]

@minxu74 You raise a good point. There should never be ivt(p) >= npcropmin unless prognostic crops are in use, so it is redundant to assert both. There are many cases throughout the code where only npcropmin is used to filter for prognostic crop types. There are also some places where crop_prog is used together with npcropmin. The flux is getting zeroed at the start of every timestep via the setvalue mechanism, so it should never show up as an uninitialized flux. Do you see some other risk with the current implementation?

[minxu74]

@thorntonpe I am not aware of any other risks with the current implementation as long as these variables are under crop_prog or ivt(p)>npcropmin if statement. Because the uninitialized errors are majorly caused by the inconsistency to handle the variables in different parts of codes. For example, they are allocated and assigned NaNs unconditionally first and are defined to zero only when crop_prog=.true. by setvalue mechanism late. Then they are assigned some values with physical meanings only when crop_prog=.true. or ivt>npcropmin, but at last they are used unconditionally in some parts of the code.

[thorntonpe]

@minxu74 I was missing that problem - thanks for clarifying. Let's decide on a fix after the new data architecture PR #2742 is merged (underway). We could remove the if (crop_prog) condition in SetValues(), or we could add conditions elsewhere.