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

SUN1999 albedo implementation in snow_utility.c #39

Closed
bartnijssen opened this issue Aug 14, 2013 · 4 comments
Closed

SUN1999 albedo implementation in snow_utility.c #39

bartnijssen opened this issue Aug 14, 2013 · 4 comments
Assignees
Labels
Milestone

Comments

@bartnijssen
Copy link
Member

The SUN1999 snow albedo implementation in snow_utility.c appears different from that in the paper mentioned in the same code (see attached figure for the relevant section).

In particular:

  • The calculated albedo is only used in Sun1999 for clear sky and is further modified in the case of cloudy skies, something that is not done in VIC
  • From the paper, it would appear that the time unit in the deep snow equation should be in hours (delta t / 3600) in the paper - unfortunately I could not quickly find the units for delta t in the paper, but it seems to be in seconds. In the VIC implementation this is converted to days
  • In the paper the cutoff for the deep snow formulation is a snow depth of 25 cm. In the VIC formulation this is 2.5 cm

I am not sure how often the SUN1999 option is actually used. But we should either
a. make sure that it is implemented correctly (which it does not seem to be)
b. remove it

If we do keep it, the comment should reflect at the very least that this is not a complete implementation of the version in the paper (and we should make sure that the units, etc are correct).

If it is wrong (as I think it is), please mark it as a bug and keep it open till fixed.

Sun, S., J. Jin, and Y. Xue, A simple snow-atmosphere-soil transfer model, J. Geophys. Res., 104 (D16), 19,587-19,597, 1999, doi:10.1029/1999jd900305.

sun1999_albedo

@ghost ghost assigned tbohn Aug 14, 2013
@bartnijssen
Copy link
Member Author

Comment from Ted Bohn per email:

I agree, it appears to be an incomplete implementation. I took that code from Keith Cherkauer many years ago. I didn't read the Sun et al paper very closely, so I missed the mismatch between the code and the paper. I should have been more cautious; however fortunately I implemented it as an option.

I suspect few people have used that option. At the time, the US ACE option was hard-wired to switch from accumulation to ablation on March 1, which made little sense in the Southern Hemisphere. So, the Sun et al option appeared to be a more process-based scheme that would be guaranteed to work globally. But in the intervening years, we've updated the US ACE scheme to use October 1 for the cut-off date in the Southern Hemisphere, so the Sun et al scheme no longer has that advantage.

I personally don't see much benefit to keeping the Sun et al option. To keep it, we would need to implement it correctly and then validate it against observations. This would take time and effort that could be spent elsewhere, for a questionable amount of improvement in the albedo scheme.

@bartnijssen
Copy link
Member Author

Agreed - since the method is incompletely implemented I would mark it for removal. This will require a few changes:

  • remove SUN1999 as an option (in the function that reads this option)
  • remove code in snow_utility.c that implements this option

I suggest that we do this as part of 4.2 as a cleanup item. If someone wants to take on implementing this correctly, we can bring it back in a future version.

I remarked it as a bug, which can be closed when the option is removed

@bartnijssen
Copy link
Member Author

Closed per pull request #45

jhamman pushed a commit that referenced this issue Aug 15, 2013
There is no longer an option to select a snow albedo implementation other than USACE

Conflicts:
	src/get_global_param.c
	src/initialize_global.c
	src/vicNl_def.h

Merge with Carbon branch
@jhamman jhamman reopened this Mar 5, 2014
@jhamman
Copy link
Member

jhamman commented Mar 5, 2014

I don't think these changes made it into the develop branch. I have removed the option in my development branch and will do the merge once the testing is complete.

jhamman pushed a commit that referenced this issue Mar 7, 2014
remove sun1999 albedo option

Resolves Issue #39
@jhamman jhamman closed this as completed Mar 7, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants