-
Notifications
You must be signed in to change notification settings - Fork 30
Replace impl_pure_record and similar macros with common Python classes #271
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
Conversation
|
Several model's parameter records have fields that are non-f64, but as long as they are de/serializable (which they all are), we can use Use
|
|
I implemented the version with Todo
|
|
I introduced from feos.eos import EquationOfState
from feos import PureRecord, Identifier, Parameters
pr = PureRecord(Identifier(), 12.0, m=1.2, sigma=3.0, epsilon_k=150, mu=2.4)
par = Parameters.from_records([pr])
eos = EquationOfState.pcsaft(par)or, e.g., par = GcParameters.from_json_smiles(["CCCCCC"], "sauer2014_smarts.json", "sauer2014_hetero.json")
eos = EquationOfState.gc_pcsaft(par)
State.critical_point(eos)Errors in the construction of the parameters will now only be recognized at the point of instantiating the eos, the There is some unpleasant redundancy in the Todos
|
PyIdentifier,PyChemicalRecordand builds the Python classes directly from the rust versions instead.impl_pure_record,impl_binary_record, andimpl_segment_recordmacros in favor of the newPyPureRecord,PyBinaryRecordandPySegmentRecordthat useIndexMap(kwargs) instead of genericsBinaryRecord<I,M>intoBinaryRecord<M>andBinarySegmentRecord.BinarySegmentRecordis (for now) not generic, but rather always usingf64as model record.No changes to the Rust interface at all. Will break all models that use non-f64 parameters (i.e., bool flags, enums, Vecs (especially with non-fixed lengths)). Therefore, the next step has to be to unify the parameter creation which I think is worth it in terms of maintainability and structure of the Python code.