EOS Van der Waals + Peng Robinson

CalebLaird commented 6 years ago

Files include EOS file for Van der Waals + Peng Robinson, VDW constants and added runtests

SimonEnsemble commented 6 years ago

Very nice! calculate_properties looks great!

can you remove Bonding.jl since you don't want to push that with this pull request yet? git rm -f Bonding.jl
can you remove the old file EOS_vdw.jl and put your vdw gas content in EOS.jl? I think only one file for equations of state is fine.
I see you changed PengRobinsonGas to PengRobinsonMolecule. This could be a gas or a liquid, so you're right that "Gas" maybe isn't the best. How about PengRobinsonFluid and VdWFluid? Just because here we already have Molecule types and these should literally characterize a molecule (position, orientation, etc.). The properties of a fluid can arise from only moles of molecules so I think we should reserve Molecule for when we keep track of its coordinates and structure in this package.
When you change the name PengRobinsonMolecule -> PengRobinsonFluid, you have to change it in src/PorousMaterials.jl. Looks like the old name PengRobinsonGas is still in there, so these tests shouldn't have even run? Be sure to run the tests before you push a pull request and do git status to make sure there aren't unpushed/uncommitted files that would prevent the tests from running. Now that Arthur has integrated tests in the cloud, hopefully this will help out!
lines 745-450, the numbers for the tests for the Peng Robinson EOS are changed. Why is that? Does this test run? Have you merged the latest version of master into your branch?

Thanks, the main vdw gas code looks great, these are just some cosmetic things. We shall put it in use over the next year!

SimonEnsemble commented 6 years ago

did you try running the tests? I get

EOS Tests: Error During Test at /home/simoncor/.julia/dev/PorousMaterials/test/eos_test.jl:11
  Got exception outside of a @test
  ArgumentError: "/home/simoncor/.julia/dev/PorousMaterials/test/data/VDW_Constants.csv" is not a valid file

is this file in the right place? please pull and try with Julia 1.0 too, as the 1./ was giving an error.

silsbyc96 commented 6 years ago

I just updated the file. It should be compatible with Julia 1.0 now. I ran the test file as well and I got ten passes out of ten.

SimonEnsemble commented 6 years ago

@silsbyc96 the build failed, see above. make a fresh clone of the github repo and make sure your files are committed, or git status.

SimonEnsemble / PorousMaterials.jl

EOS Van der Waals + Peng Robinson #41