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

Atmospheric Stability formula correction for standard method #521

Closed
wants to merge 2 commits into from

Conversation

iamtekson
Copy link

Make sure all the relevant boxes are checked (and only check the box if you actually completed the step):

  • Closes #xxx (identify the issue associated with this PR)
  • Code passes standard test cases (results are either bit-for-bit identical, or differences are explained in the PR comment)
  • New tests added (describe which tests were performed to test the changes)
  • Science test figures (add figures to PR comment and describe the tests)
  • Checked that the new code conforms to the SUMMA coding conventions
  • Describe the change in the release notes (use either ./summa/docs/whats-new.md or ./summa/docs/minor-changes.md depending on what changed)

I got stabality correction formula as below in the Anderson (1976) paper.

image

@wknoben
Copy link
Collaborator

wknoben commented Apr 19, 2023

Hi, thanks for the PR!

Can I please ask you to update the following?

  • We typically don't merge directly into master. Instead, new changes are based on and merged into the develop branch. When enough changes accumulate there we move them into a new stable release on the master branch (see: https://summa.readthedocs.io/en/latest/development/SUMMA_git_workflow/). Could you re-open this PR using develop as the base and target branch?
  • I assume there was a reason why you started digging into this in the first place and it would be good to know what that reason was. Could you add a brief description (possibly including a plot) that shows what results you got before the fix and why that seemed wrong to you?

@iamtekson
Copy link
Author

iamtekson commented Apr 19, 2023

Hi @wknoben, I was working on a Capstone project from GEOG 825 course. We are doing sensitivity analysis testing using SUMMA software. While I was writing the report, I was comparing the flux equation on SUMMA implementation as well as on the actual paper. I found the stability correction formula for stable method different in Anderson (1976) paper and tried to modify it. But when I discuss with Dr. Clark, he said that, most of the equations were taken from Choudhury and Monteith (1988).

I think if that was the case, I only suggest you to change the comment and link the associate paper in this method. For now, you can close this PR without merging it.

PS: In atmospheric correction, I found sensitivity of the standard method relatively very sensitive to other method.

image

@wknoben
Copy link
Collaborator

wknoben commented Apr 20, 2023

Hi Tek,
Thanks for the clarification. I'll close this but have recorded the request for clearer comments in #522

@wknoben wknoben closed this Apr 20, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants