Integrated impl_cubics features into main

[JelleLagerweij]

As per request by @g-bauer, I created a new pull request comparing to main.
Jelle

For reference, original pull request text is below

---

Dear all,
I forked the feos main repository and copied all the added functions in impl_cubics to the new structure. During this project the following major changes are made:

Project level:

.github/.workflows/test.yaml now automatically test the cubic feature as well.
Cargo.toml also depends on enum_dispatch (this was not there in the main branch, but usedin the impl_cubics branch)

Feos create level:

crates/feos/Cargo.toml now also depends on the serde feature of ndarray. This is because serde::Serialize is not implemented for ndarray::Array1 without this feature (we use this in crates/feos/cubic/alpha/soave.rs lines 10-14). I think we could use a Vec instead ndarray::Array1 instead if we want to avoid this dependency.
All Error and Result types/enums now are redirected to FeosResult and FeosError.

Next steps:
While the code compiles correctly now, there are definetly some tasks left.

we do not pass the a_res test, the error is small though. The test compares the output of feos-core/src/cubic (the Peng-Robinson example used in the tutorial) with the implementation here. I will need to figure out where the difference comes from later.
I want to create a test to test the mixture predictions of the package (maybe I will mix Propane and Methane or something similar and easy). In the issue, you indicated that there might be some changes required to the namings of the mixture functions. Can you indicate them, then I can implement those before adding the test.

Regards,
Jelle

[JelleLagerweij]

Sorry, without the 30k changes, I already can see some problems in my pull request:
some .vscode file should not be in there, and somehow there is some small change in the imports in the pets and epcsaft modules. I looked into them to understand the code structure better, but I guess that the rustfmt restructured the input to be on alphabetical order on saving the file.

use quantity::{GRAM, MOL, MolarWeight};

I will first remove those before submitting a new pull request and, I will be more careful with this in the future (so there are less useless pull requests in this repository).

[JelleLagerweij]

Lol its clear that I mostly work on my own projects. Now it should be a proper pull request with only changes regarding what I am responsible for. :)

[g-bauer]

Hi Jelle and thanks again for the contribution. If I may, here is a tip for the usual workflow when contributing to projects on Github:

• Fork the repo (leave the name as-is)
• Create a branch (based on the origin's main or a branch) in your fork and implement the features you want to contribute in that branch (you can name the branch in accordance with the feature you are implementing which makes it easier to find stuff when you have multiple branches)
• Once you are done (or need feedback), you can create a PR (draft) in the origin's repository. You can do that very easily from your fork.
• You can then add additional commits in your fork's branch and they are automatically added to the PR's commits.

How branches are used is different in projects, but usually we create a branch per feature (Philipp and I can do that directly in this repo, contributors have to fork).

For this PR, you can leave it as is.

---

Regarding the actual implementation: I think the next step would be to add tests as you mentioned. Ideally, for all alpha functions (I didn't test them in my implementation back then). I have to check for a better/correct naming for the mixing-rule but this is a simple refactor that can be done at any point.

[prehner]

Hey there,
sorry for adding the additional confusion with the parameter rework. I hope in the end it makes things simpler, but you were caught in the middle of the process. For you I recommend the following steps:

• in cubic/parameters you can remove a couple of things:
  • #[derive(Default)] for CubicRecord (we had this in other places, but it doesn't really make sense for pure component parametesr)
  • all impl Display (printing of PureRecord is done via serialization)
  • From<f64> for CubicBinaryRecord and From<CubicBinaryRecord> for f64 (that was a bit of a hack that we decided to get rid off)
  • new_simple (we use something like this somewhere for tests, but for maintainability, I prefer that everything uses the same interface)
  • molarweight, pure_records and binary_records in CubicPars (the CubicPars struct is just for convenience, the parameters are all stored in CubicParameters)
• Regarding this CubicPars struct: First of all, sorry for the confusing naming here. I used something like this in the more complex models like PC-SAFT. For the cubics, my suggestion is to move the fields (tc, pc, acentric_factor, k_ij, l_ij) directly into Cubic. If you think it turns out more clear if you have an extra struct, that is also fine.
• In cubic/mod.rs:
  • remove all the Arcs around CubicParameters
  • Use the collate and collate_binary methods of CubicParameters to obtain the fields of CubicPars (wherever they are stored)
  • Again, you can remove the implementation of Display

If anything is unclear, just let us know.

Sorry again for the extra confusion and thank you for your efforts in tackling this comprehensive implementation of cubics!

[JelleLagerweij]

Hi,
I haven't spend much on this project lately, but definitely intent to continue.

I was having a bit more time this weekend and spend some hours on the cubic implementation. As I saw that there were updates in the main repository, I tried to merge these first with my cubic branch. The merging went fine, however I encountered a problem with the Parameter -> Parameters change.

If I got it correctly, instead of creating a struct for CubicParameters and implement that for Parameter, we will now create a pub type CubicParameters = Parameters<CubicRecord, CubicBinaryRecord, ()>; (no association term required for cubic). I encountered problems with the impl CubicParameters {}, and after reading the documentation I figured out that this was fine for the cubic.rs in feos-core as that is in the same crate as the Parameters definition. While our cubic module is located in another (sub)crate feos.

So i looked more into the saftvrmie implementation there we do

pub type SaftVRMieParameters =
    Parameters<SaftVRMieRecord, SaftVRMieBinaryRecord, SaftVRMieAssociationRecord>;
 
 pub struct SaftVRMiePars {
    pub m: Array1<f64>,
    pub sigma: Array1<f64>,
    pub epsilon_k: Array1<f64>,
    pub lr: Array1<f64>,
    pub la: Array1<f64>,
    pub sigma_ij: Array2<f64>,
    pub epsilon_k_ij: Array2<f64>,
    pub lr_ij: Array2<f64>,
    pub la_ij: Array2<f64>,
    pub c_ij: Array2<f64>,
    pub alpha_ij: Array2<f64>,
}

impl ...

First I thought that this was a simple trick to avoid the impl issue for types outside the its containing crate by creating a sort of shadow struct which then depends fully on the SaftVRMieParameters. However lateron I saw that both (SaftVRMieParameters and SaftVRMiePars) were used in the saftvrmie equation of state module.

So for the Cubics implementation: We will an approach similar to saftvrmie:

pub type CubicParameters = Parameters<CubicRecord, CubicBinaryRecord, ()>;

pub struct CubicPars {
    ...
}

impl CubicPars {
    pub fn new(parameters: &CubicParameters) -> Self {
    ...
    }
}

And in mod.rs we will now have cubic

pub struct Cubic {
    /// Parameters
    pub parameters: Arc<CubicParameters>,
    pub params: Arc<CubicPars>,
    pub options: CubicOptions,
    /// processed parameters using model and substance critical data
    pub critical_parameters: CriticalParameters,
}

However, what properties SHOULD be in CubicParameters and what SHOULD be in CubicPars? I guess that the CubicParameters are mostly useful for guaranteeing that the eos parameters are valid (and can be automated with reading from a .json file).

[prehner]

Hi, just checking, because I'm not sure, you saw my previous post (we posted somewhat simultaneously). That should explain most of your questions around the Pars structs? The naming is poor, I apologize for that :D but it is also something that is only used locally to store some variables in convenient places. The CubicParameters type alias is the one that appears on the outside, when you actually instantiate your model.