Skip to content
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

Feature/update organic fract option #837

Merged

Conversation

dgergel
Copy link
Contributor

@dgergel dgergel commented Oct 1, 2018

This PR enables the ORGANIC_FRACT option to be turned on in VIC 5 (it was tested as part of this PR) and also enables the BULK_DENSITY_COMB option to be used in conjunction with it.

d3start, d3count, dvar);
for (i = 0; i < local_domain.ncells_active; i++) {
soil_con[i].bulk_dens_org[j] = (double) dvar[i];
}
Copy link
Member

Choose a reason for hiding this comment

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

what happens if BULK_DENSITY_COMB == True? Are you sure we're parsing BULK_DENSITY_COMB and ORGANIC_FRACT properly from the global parameter file?

Copy link
Contributor Author

@dgergel dgergel Oct 2, 2018

Choose a reason for hiding this comment

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

@jhamman yes. If BULK_DENSITY_COMB == True, then we read in the combined bulk density (mineral and organic) from the parameter file rather than reading in the organic bulk density separately.

What I don't know is to what extent reading in the combined bulk density changes soil temperatures vis-a-vis turning on the ORGANIC_FRACT option, but I could test that and I think it would be useful to understand. Reading in the combined bulk density without turning on ORGANIC_FRACT definitely impacts soil temperatures. As @bartnijssen and I have discussed, the bulk density is used throughout the frozen soils routine, so just using the combined bulk density without turning on ORGANIC_FRACT is already incorporating the organic content of the soil into the thermal conductivity calculations.

Copy link
Member

Choose a reason for hiding this comment

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

Would be good to see a test. Can be done for a single grid cell.

Copy link
Member

@bartnijssen bartnijssen left a comment

Choose a reason for hiding this comment

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

Approved - but would be good to see the test before we merge

@dgergel
Copy link
Contributor Author

dgergel commented Oct 3, 2018

I just made an issue for doing some follow-up analysis that will look at the impact on soil temperatures of turning on the ORGANIC_FRACT option. See #844 for reference

@bartnijssen
Copy link
Member

@dgergel : If you resolve the conflict then I can merge

@dgergel
Copy link
Contributor Author

dgergel commented Oct 3, 2018

@bartnijssen resolved the conflict earlier, this should be ready to merge

@bartnijssen bartnijssen merged commit 77133b0 into UW-Hydro:develop Oct 9, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants