-
Notifications
You must be signed in to change notification settings - Fork 415
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
Fix to state file i/o problems and refactoring of initialization #464
Conversation
… separate functions for generating a default state and for handling derived state variables.
Note: I've tested that both classic and image drivers now successfully save state at the appointed time, and either read the specified state file or start from a default state as requested. Note: results are a little different between the new classic and 4.2. Not hugely, but still, not the same. Hopefully this can be investigated further in restart tests. |
Another note: strangely, starting from the the default state does not generate root_brent errors anymore after this PR is merged. However, starting from a state file still does. Not sure why. |
@@ -692,6 +696,108 @@ read_soilparam(FILE *soilparam, | |||
temp.Ws = temp.Ws / temp.max_moist[layer]; | |||
} | |||
|
|||
// Soil thermal node thicknesses and positions |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This section was originally in the old initialize_model_state()
function, but it really should go here, as node spacing is completely determined from soil parameters and global param options. This way, all soil_con
values are populated by the time we exit read_soilparam()
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It seems that currently the last section (around the last ~15 lines) in initialize_soil.c
also derives some soil_con
values. Should that part be moved here to read_soilparam()
also?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, I think you are right. Good catch. I need to move those into both read_soilparam() and vic_init().
} | ||
} | ||
|
||
return(0); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we've been moving away from returning an exit status unless it will be used later. In this case, we should exit immediately if there are any errors and we can return with void
instead.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK
* This routine initializes the model state (energy balance, water balance, and | ||
* snow components). | ||
* | ||
* If a state file is provided to the model than its |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"then" instead of "than"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK
@tbohn : merge conflicts (probably because of Joe's PR that was just merged) - please merge |
Conflicts: vic/drivers/shared_image/include/vic_driver_shared_image.h vic/drivers/shared_image/src/vic_store.c
Fix to state file i/o problems and refactoring of initialization into separate functions for generating a default state and for handling derived state variables. Most of these functions are shared across drivers.
closes #457
closes #424
closes #401
Quick summary of the changes:
New structure:
read_soilparam(classic) or vic_init(image/cesm):
vic_populate_model_state(all drivers):