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

Modified downwelling sfc flux calculation #109

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

helgehr
Copy link
Contributor

@helgehr helgehr commented Oct 30, 2023

Thanks for this amazing resource!
I modified the flux computation (only cells 33/34/35 are changed) in this notebook because I noticed that it was not consistent with the definition of the signs of up/downwelling fluxes. Because the signs where flipped for both sides of the equation in cell [34], the result at the end was correct before.
I don't know why so many output cells are marked as changed, if you give me a hint how I can avoid that, I could open a more "clean" pull request.

@review-notebook-app
Copy link

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

@github-actions
Copy link

github-actions bot commented Oct 30, 2023

👋 Thanks for opening this PR! The book will be automatically built with GitHub Actions. To see the status of your deployment, click below.
🔍 Git commit SHA: a1f63c2
✅ Deployment Preview URL: https://brian-rose.github.io/ClimateLaboratoryBook/_preview/109

@brian-rose
Copy link
Owner

Thanks!

I think the only way to get a more "clean" diff would be to strip all the output of the notebook before committing to version control. That would work well if all the notebooks were lightweight and the output could easily be regenerated, but that's not the case for all the notebooks in the repository.

So don't worry about it.

I will take a look at the sign issue when I have a bit of time. I appreciate the contribution!

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