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

Add validity ranges and clean up water_prop_pack, seawater_prop_pack, and NaCl_T_dep_prop_pack #1304

Merged
merged 23 commits into from
Mar 22, 2024

Conversation

savannahsakhai
Copy link
Contributor

@savannahsakhai savannahsakhai commented Feb 19, 2024

Fixes/Resolves:

Summary/Motivation:

This PR cleans up three of the property models by reformatting the in-code citations and adding validity ranges for each of the properties.

Changes proposed in this PR:

  • Update the references for the NaCl_T_dep_prop_pack in the documentation
  • Add the reference list to NaCl_T_dep_prop_pack
  • Reformat citations and add validity ranges to water_prop_pack and seawater_prop_pack
  • Remove commented out or unnecessary code from property functions in the water_prop_pack (that had been adapted from the seawater_prop_pack)

Legal Acknowledgement

By contributing to this software project, I agree to the following terms and conditions for my contribution:

  1. I agree my contributions are submitted under the license terms described in the LICENSE.txt file at the top level of this directory.
  2. I represent I am authorized to make the contributions and grant the license. If my employer has rights to intellectual property that includes these contributions, I represent that I have received permission to make contributions and grant the required license on behalf of that employer.

@savannahsakhai savannahsakhai self-assigned this Feb 19, 2024
Copy link

codecov bot commented Feb 19, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 94.25%. Comparing base (492e8db) to head (b8500e3).

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1304      +/-   ##
==========================================
- Coverage   94.25%   94.25%   -0.01%     
==========================================
  Files         370      370              
  Lines       37951    37950       -1     
==========================================
- Hits        35769    35768       -1     
  Misses       2182     2182              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@ksbeattie ksbeattie added the Priority:Normal Normal Priority Issue or PR label Feb 22, 2024
@savannahsakhai savannahsakhai changed the title Add validity ranges and clean-up water_prop_pack, seawater_prop_pack, and NaCl_T_dep_prop_pack Add validity ranges and clean up water_prop_pack, seawater_prop_pack, and NaCl_T_dep_prop_pack Feb 26, 2024
Co-authored-by: Adam Atia <aatia@keylogic.com>
@adam-a-a
Copy link
Contributor

@savannahsakhai I think we can reserve the inclusion of saturation temperature and temperature-dependent diffusivity in a separate PR since we are prepping for a release and can get these straightforward changes in in the meantime.

Copy link
Contributor

@hunterbarber hunterbarber left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changes to comment clean up are good. Convert any feature requests to corresponding issues.

Copy link
Contributor

@ElmiraShamlou ElmiraShamlou left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@lbianchi-lbl lbianchi-lbl enabled auto-merge (squash) March 22, 2024 15:21
@lbianchi-lbl lbianchi-lbl merged commit 32af858 into watertap-org:main Mar 22, 2024
24 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Priority:Normal Normal Priority Issue or PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants