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

Use p_vap_sat func in q_vap_sat #123

Merged
merged 1 commit into from
Aug 8, 2022
Merged

Use p_vap_sat func in q_vap_sat #123

merged 1 commit into from
Aug 8, 2022

Conversation

charleskawczynski
Copy link
Member

This PR is a small refactoring step towards #119. This PR:

  • Slightly generalizes saturation_vapor_pressure to allow users to provide a phase partition
  • saturation_vapor_pressure is then called inside q_vap_saturation, whose contents were moved over

This mostly just deletes duplicate code. There is a bit of algebraic difference between the two, but I've confirmed that they're equivalent.

@codecov
Copy link

codecov bot commented Aug 8, 2022

Codecov Report

Merging #123 (8984cd3) into main (b061849) will decrease coverage by 0.04%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main     #123      +/-   ##
==========================================
- Coverage   93.88%   93.83%   -0.05%     
==========================================
  Files           8        8              
  Lines         997      989       -8     
==========================================
- Hits          936      928       -8     
  Misses         61       61              
Impacted Files Coverage Δ
src/relations.jl 96.88% <100.00%> (-0.05%) ⬇️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@charleskawczynski
Copy link
Member Author

I've looked over this once more, and I realize now that we have correctness tests that cover this, so merging!

@charleskawczynski
Copy link
Member Author

bors r+

@bors bors bot merged commit e6eaddc into main Aug 8, 2022
@bors bors bot deleted the ck/refactor2 branch August 8, 2022 23:50
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.

1 participant