Skip to content

fix: volon_subtimestep_cm used unitialized when flux caching is enabled#47

Open
hellkite500 wants to merge 1 commit intoNOAA-OWP:masterfrom
hellkite500:fix-uninitialized
Open

fix: volon_subtimestep_cm used unitialized when flux caching is enabled#47
hellkite500 wants to merge 1 commit intoNOAA-OWP:masterfrom
hellkite500:fix-uninitialized

Conversation

@hellkite500
Copy link
Copy Markdown
Contributor

Bug fix that can lead to garbage results and mass balance when variable isn't correctly initialized but used in the flux caching branch.

@robertbartel
Copy link
Copy Markdown
Contributor

FYI, the build failures for this should be resolved once #48 is approved and merged, and then this PR branch is rebased on top of the subsequent state of master.

AET_subtimestep_cm = 0.0;
volstart_subtimestep_cm = 0.0;
volin_subtimestep_cm = 0.0;
volon_subtimestep_cm = 0.0;
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I am thinking if the mass balance issue due to the garbage surface water, is not directly caused by volon_subtimestep_cm initialization. As it gets updated at the following lines:

volon_subtimestep_cm = ponded_depth_subtimestep_cm;

volon_subtimestep_cm = ponded_depth_subtimestep_cm;

volon_subtimestep_cm = ponded_depth_max_cm;

Austin confirmed that ponded_depth_max is set to 0.0 in the config file. So let's look at the first two.
The ponded_depth_subtimestep_cm is calculated at line using precip_subtimestep_cm_per_h and then at line using volon_timestep_cm.

My point is maybe the volon_timestep_cm is the culprit and not volon_subtimestep_cm. If not then, probably the problem is in the function lgar_create_surficial_front which updates ponded_depth_subtimstep

If I am not clear, let's chat about it.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

You are correct that it gets initialized on those lines, but ALL of those are nested under this conditional

if (!state->lgar_mass_balance.cache_fluxes)

When flux caching is enabled, this block of code executes instead and the variable in question is truly uninitialized when used in the local_mb calculation.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants