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/fix deltacc bug #776

Merged
merged 19 commits into from Feb 8, 2018

Conversation

Projects
None yet
3 participants
@dgergel
Contributor

dgergel commented Feb 1, 2018

  • closes #775
  • tests passed
  • new tests added
  • science test figures
  • ran uncrustify prior to final commit
  • ReleaseNotes entry

This PR fixes the issue described in #775. In addition to what is described there, the unreasonable deltaCC values were a result of incorrect values for the ground flux out of the snowpack, which is subtracted from the deltaCC term. Calculating the ground flux out of the snowpack depends on snow density (squared) and snow depth, and extremely large values for snow density were occurring for certain grid cells at certain time steps in the RASM domain. This was fixed by a) implementing a cap on the new snow density calculated by the Hedstrom and Pomeroy 1998 equation, and b) disregarding the ground flux out of the snowpack if snow depth is below a certain threshold (1.e-8). A new parameter, SNOW_NEW_SNOW_DENS_MAX, was added to the parameters struct to cap snow density.

@dgergel dgergel self-assigned this Feb 1, 2018

@dgergel dgergel requested review from bartnijssen and jhamman Feb 1, 2018

dgergel added some commits Feb 1, 2018

@bartnijssen

This comment has been minimized.

Show comment
Hide comment
@bartnijssen

bartnijssen Feb 6, 2018

Member

Looks good to me : @jhamman - another pair of eyes may be useful. Feel free to merge.

Member

bartnijssen commented Feb 6, 2018

Looks good to me : @jhamman - another pair of eyes may be useful. Feel free to merge.

@jhamman

Just one small comment to move the hard coded literal out of vic_run.

Show outdated Hide outdated vic/vic_run/src/SnowPackEnergyBalance.c
@dgergel

This comment has been minimized.

Show comment
Hide comment
@dgergel

dgergel Feb 7, 2018

Contributor

@jhamman - I've updated this to add the snow depth threshold as a parameter rather than a hard coded literal. Ready for another review

Contributor

dgergel commented Feb 7, 2018

@jhamman - I've updated this to add the snow depth threshold as a parameter rather than a hard coded literal. Ready for another review

@jhamman jhamman merged commit 8f2f568 into UW-Hydro:develop Feb 8, 2018

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment