-
Notifications
You must be signed in to change notification settings - Fork 92
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
patch and cohort initialization needs love #231
Comments
Revisiting this issue as @rgknox and I have had some discussion about cleaning up the extensive NaN followed by zeroing of variables (for instance in EDCohortDynamics: fates/biogeochem/EDCohortDynamicsMod.F90 Line 436 in 3f6749d
fates/biogeochem/EDCohortDynamicsMod.F90 Line 546 in 3f6749d
on the question of NaN and zeroing of variables @rgknox recommended this: Regarding zero'ing. I think we should only be zero'ing a variable when there is a meaningful intention behind it. If a variable is going to be incremented over a loop, then I would zero it right before the loop, and only the indices in the loop. If a variable is going to be incremented over a few subroutines, then maybe create a very visible subroutine to zero the variable before those subroutines, and group it with others doing the same thing. But, if a variable is not incremented, I would avoid zero'ing, because this will make it hard to determine later in the code, if the variable is used, if a value of zero means its really zero, or was just ignored or missed." |
I agree, I'm not attached to the zeroing situation... Going with NaNs seems like a more robust way to catch errors... |
We have reoriented how we newly define cohort variables in recent times, following these ideas. I don't see a massive refactor on how we create cohorts coming anytime soon though. Lets close and re-evaluated someday if need be. |
The current method of creating new patches and cohorts seems to be to something close to the following:
I'm not to thrilled with part 3. I would rather us initialize everything with a NaN following the mold of CLM/ALM, and use the CIME library definition of nan.
Then we can have a section where we target specific variables that should be zero'd (or other constants), and give the scientifically justifiable reason for it in each case. I really don't want to zero a variable that is intended to be over-written later, only for it to slip through the cracks, not receive the correct initialization, and that zero to allow the model to keep plugging along and use that bogus value.
Of course, some initial conditions are passed in as arguments, and should remain that way.
The text was updated successfully, but these errors were encountered: