search

feos-org / feos

FeOs - A Framework for Equations of State and Classical Density Functional Theory
Other
108 stars 22 forks source link

QoL improvements for working with ideal gas models #203

Closed prehner closed 6 months ago

prehner commented 7 months ago

The PR contains four smaller proposed changes that are each open to discussion individually.

  1. The molarweight field is removed from PureRecord and added to PcSaftRecord instead (the other eos would have to be updated as well)
  2. A new NoResidual struct is used (internally) to be able to construct an EquationOfState with only an ideal gas model, e.g. #204 
    EquationOfState.ideal_gas().joback(joback)
  3. The Arc around the ideal gas part in EquationOfState is removed. The corresponding Arc around the residual part is required exactly once for the "building" pattern for EquationOfState in Python. I would prefer to also get rid of that, because it is unnecessary for Rust and conceptually not required at that point. That would require a different constructor for EquationOfState in Python, though.
  4. PureRecord, SegmentRecord, Identifier, and IdentifierOption are added to feos.ideal_gas. This is related to a larger issue that there are multiple PureRecords and SegmentRecords in the Python package. Identifier and IdentifierOption could be exposed only once at a central location. #205

The changes in 1. and 3. are breaking changes, but 2. is a nice quality of life improvement that could be published as patch. 4. more of a fix, but does not really solve the root of the problem.

g-bauer commented 7 months ago
  1. makes sense since we removed the trait and made it a method of Residual .
  2. fine with me
  3. Pythonic initialization was the reason for Arc back then - otherwise building the EoS gets awkward in Python. I don't mind the Arc in Rust too much (it is not slowing down the code). If we decide to remove them, maybe we should write a single constructor for an equation of state and work with a "configuration" object to provide parameters and options for EoS instead of a method for each model?