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

QoL improvements for working with ideal gas models #203

Closed
wants to merge 5 commits into from

Conversation

prehner
Copy link
Contributor

@prehner prehner commented Nov 28, 2023

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. EquationOfState without residual model #204
EquationOfState.ideal_gas().joback(joback)
  1. 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.
  2. 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. Add PureRecord and other Python classes to feos.ideal_gas #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
Copy link
Contributor

g-bauer commented Nov 28, 2023

  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?

@prehner prehner closed this Dec 18, 2023
@prehner prehner deleted the ideal_gas_finetuning branch December 18, 2023 14:21
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.

2 participants