pmgbergen / porepy

Python Simulation Tool for Fractured and Deformable Porous Media
GNU General Public License v3.0
251 stars 88 forks source link

CF merger #2: introducing `porepy.compositional` #1186

Closed vlipovac closed 5 months ago

vlipovac commented 5 months ago

Proposed changes

Introducing modeling of multicomponent and multiphase mixtures. Introducing also mixin classes which enables user to easily create mixtures and introduce a attribute fluid_mixture into the model.

Things to discuss:

  1. I prepared a test for singular mixtures, testing that unnecessary variables are not created in the case of 1 component or 1 phase or both. But this is non-trivial, if we allow phases to have different components (so not every component has to be in every phase). I did not make any restrictions here, since this "unified assumption" is only necessary in the unified flash. I need your thoughts on this.
  2. Integration with models and the existing fluid constants: Two options: a. make functionality to build a single-phase, single-component mixture with some simplified EoS from fluid constants. b. replace fluid with the fluid mixture and start transitioning the model framework to a more general setting. On a side node, a similar discussion will be required when introducing the compositional flow equations (relation to current single phase flow etc).

Types of changes

Unrelated changes: Overload of logical operators in forward_mode.AdArray. I think this is required in some point of the new code. Bugfix: Fixing memory leak in surrogate operator (require depth parameter for shifting values in time and iterate sense, otherwise excessive storage)

Checklist

For the record

This PR contains some fixes in numba-compiled functions (related to casting and signature typing), which were discovered in some completely unrelated context.

vlipovac commented 5 months ago

@keileg

2 minor things I want to draw your attention to, which you might miss in your review.

https://github.com/pmgbergen/porepy/blob/f6dcb96d86f4e5c20223dce13afd1d2cd6bd356b/src/porepy/compositional/__init__.py#L50 https://github.com/pmgbergen/porepy/blob/f6dcb96d86f4e5c20223dce13afd1d2cd6bd356b/src/porepy/compositional/base.py#L99

vlipovac commented 5 months ago

@keileg I have added tests for the utility functions in tests/compositional/test_utils.py

Could you please go through all unresolved conversations and mark them resolved if you are satisfied with the changes? All of them should be covered by the changes, except the discussion on the changes in requirements.txt We discussed this in person mostly. Regarding the dependency on chemicals, I have no strong opinion. If you want it to be a weak dependency, I only have to change some tests where I (for convenience) created components using the API function.

vlipovac commented 5 months ago

@keileg chemicals is now a weak requirement. If the user installs it, the method load_species is usable. The subpackage works without it, tests were adapted to not rely on load_species.