simphony / simphony-common

The native implementation of the Simphony cuds objects
BSD 2-Clause "Simplified" License
7 stars 5 forks source link

Reduced equation of state -tools #297

Closed kemattil closed 8 years ago

kemattil commented 8 years ago

Motivation/Problem Two phase (liquid-vapour) fluid flow simulations need to be initialized with good/sensible values for the density/pressure fields. These values depend on the equation of state chosen for the modeled fluid (a materials relation) as well as on the system temperature (a system parameter).

Note This is a general modelling problem (theory of thermodynamics), not a particular difficulty of, e.g., JYULB-solver.

Solution Implementation of a (general) tool which computes liquid-vapour densities based on the given equation of state and system temperature.

Features

Dependencies

@tuopuu @kitchoi @mehdisadeghi @roigcarlo please have a look, if possible.

tuopuu commented 8 years ago

@kemattil, We should find out what's the problem with travis-ci in this branch, and fix it. Other than that, I would like to change most of the parameter descriptions in the methods in the format we've been using in SimPhoNy:

parameter_name : type
                 Descriptive textual definition

I understand that in most cases in this branch, the type would be just a float, but it's better to keep documentation style consistent throughout the project.

About SciPy dependency: I think it's unlikely that simphony-common itself would need SciPy, so we might want to separate it in dev_requirements.txt file. It's likely that when we start adding more physics related tools to simphony/tools we have to add other dependencies as well (such as linear algebra libraries etc). I suggest that all dependencies of simphony/tools would be grouped together either in a separate file, or below a comment line (something like # Requirement for simphony/tools ) in dev_requirements.txt.

tuopuu commented 8 years ago

@kemattil, we had a discussion in the DevTeam meeting about adding tools to SimPhoNy and decided to build a separate repository (simphony-tools) for those. This way we avoid cluttering simphony-common requirements list with packages that are very specific to certain tools and are not needed elsewhere inside SimPhoNy.

Do you mind if I take control of this branch and add it there?

kemattil commented 8 years ago

@tuopuu thanks for the comments!

A separate simphony-tools repository is a good idea.

Take control of the branch by all means!

codecov-io commented 8 years ago

Current coverage is 97.98%

Merging #297 into master will decrease coverage by 0.41%

@@             master       #297   diff @@
==========================================
  Files            49         51     +2   
  Lines          3821       3974   +153   
  Methods           0          0          
  Messages          0          0          
  Branches        500        508     +8   
==========================================
+ Hits           3760       3894   +134   
- Misses           39         55    +16   
- Partials         22         25     +3   

Powered by Codecov. Last updated by 413eb6f...a2e8b63

kemattil commented 8 years ago

@tuopuu I made the changes to parameter descriptions as you suggested.

Still do not know why the the Travis CI fails ...

tuopuu commented 8 years ago

Thanks @kemattil. Code and descriptions now look good. It seems that AppVeyor error is caused by SciPy not installed in AppVeyor script, which we can fix when moving these files to simphony-tools repository. Travis-CI fail seems unrelated to this branch.

kemattil commented 8 years ago

Ok, good.

kitchoi commented 8 years ago

@tuopuu @kemattil The Travis CI failure is due to failing to automatically generate the documentation using the docstrings provided. There are a number of long docstrings in this branch which might cause problems for sphinx. Please try python setup.py build_sphinx to test locally if the doc can be generated successfully.

tuopuu commented 8 years ago

This branch has been moved to simphony-tools repository.