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

Update cosmosac #204

Merged
merged 6 commits into from
Sep 22, 2023
Merged

Update cosmosac #204

merged 6 commits into from
Sep 22, 2023

Conversation

pw0908
Copy link
Member

@pw0908 pw0908 commented Sep 19, 2023

Added functionalities where, if COSMO-SAC parameters are not in the Clapeyron database, they will be looked up in the NIST/COSMOSAC database.

@ianhbell do you have any opposition to this? We can add the relevant references to the model to ensure both your database and the folks who built the database receive the appropriate recognition.

@codecov
Copy link

codecov bot commented Sep 20, 2023

Codecov Report

Merging #204 (a28701e) into master (e53d32a) will decrease coverage by 1.14%.
Report is 58 commits behind head on master.
The diff coverage is 46.06%.

@@            Coverage Diff             @@
##           master     #204      +/-   ##
==========================================
- Coverage   83.16%   82.02%   -1.14%     
==========================================
  Files         251      258       +7     
  Lines       16150    16572     +422     
==========================================
+ Hits        13431    13594     +163     
- Misses       2719     2978     +259     
Files Changed Coverage Δ
ext/ClapeyronCoolPropExt.jl 11.76% <0.00%> (-88.24%) ⬇️
src/Clapeyron.jl 100.00% <ø> (ø)
src/models/Activity/COSMOSAC/utils.jl 62.50% <0.00%> (-28.41%) ⬇️
src/models/EmpiricHelmholtz/SingleFluid/parser.jl 77.19% <ø> (ø)
src/models/cubic/PR/variants/PR78.jl 100.00% <ø> (ø)
src/models/cubic/PR/variants/UMRPR.jl 100.00% <ø> (ø)
src/models/cubic/PR/variants/VTPR.jl 100.00% <ø> (ø)
src/models/cubic/RK/variants/tcRK.jl 0.00% <0.00%> (ø)
src/models/cubic/alphas/BM.jl 95.65% <ø> (ø)
src/models/cubic/alphas/MT.jl 50.00% <ø> (ø)
... and 30 more

@ianhbell
Copy link
Contributor

That makes sense to me. I would recommend a strong comment in the docs, perhaps a print statement in the console that you must manually silence, indicating the license terms within which the COSMO-SAC files are made available. They are not allowed to be used for all purposes.

@pw0908
Copy link
Member Author

pw0908 commented Sep 21, 2023

Great! I've changed the implementation so that users must intentionally select the nist database to use it, at which point a warning is thrown to let them know to check the license.

The docs themselves are getting an overhaul on a separate branch where I've added similar warnings.

@longemen3000
Copy link
Member

test fail is unrelated

@pw0908 pw0908 merged commit cce72a2 into master Sep 22, 2023
19 of 22 checks passed
@pw0908 pw0908 deleted the update-cosmosac branch September 28, 2023 05:35
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

3 participants