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

Revised EOS #113

Merged
merged 16 commits into from Jul 16, 2019
Merged

Revised EOS #113

merged 16 commits into from Jul 16, 2019

Conversation

huynmela
Copy link
Collaborator

Revised EOS script to include VdW EOS along with modified VdW properties csv. Changed gas to fluid and modified docs to include VdW.

@coveralls
Copy link

coveralls commented Jul 11, 2019

Pull Request Test Coverage Report for Build 432

  • 36 of 46 (78.26%) changed or added relevant lines in 2 files are covered.
  • 44 unchanged lines in 5 files lost coverage.
  • Overall coverage increased (+0.03%) to 79.958%

Changes Missing Coverage Covered Lines Changed/Added Lines %
src/EOS.jl 34 44 77.27%
Files with Coverage Reduction New Missed Lines %
src/EOS.jl 1 73.33%
src/Crystal.jl 3 66.9%
src/Grid.jl 4 77.73%
src/Henry.jl 5 82.2%
src/GCMC.jl 31 62.02%
Totals Coverage Status
Change from base Build 407: 0.03%
Covered Lines: 1137
Relevant Lines: 1422

💛 - Coveralls

@ahyork
Copy link
Collaborator

ahyork commented Jul 11, 2019

This had errors when running with julia 0.7 on travis build 426 that were not present in julia 1.0. I dropped support for julia 0.7 with this branch and added testing for julia 1.1

@SimonEnsemble
Copy link
Owner

@huynmela looks great. Before merging, please:

  • remove "***NOTE: Do not delete the last three comment lines..." from the doc strings.
  • Please allow CSV to read in the .csv files and skip the lines marked as a comment #. See here for how to do so.
  • Please put the source of where you got the PengRobinson and VdW params inside the .csv files. It is better here as opposed to the documentation because we should let the user amend these with their own comments and with params from different sources (negating the docs).
  • ask @ahyork for help with this: to ensure that CSV is up to date to allow reading in comments; I believe this is a recent development.

@SimonEnsemble
Copy link
Owner

when you put CSV 0.5.9 in the REQUIRE file, does that mean CSV.jl has to be 0.5.9 version, or >0.5.9 is okay?

@ahyork
Copy link
Collaborator

ahyork commented Jul 15, 2019

Having CSV 0.5.9 sets a floor for CSV at version 0.5.9, so anything >0.5.9 will also work

@SimonEnsemble SimonEnsemble merged commit 911db0d into master Jul 16, 2019
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

4 participants