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

Surprising default combining rule in association term #258

Open
ianhbell opened this issue Feb 23, 2024 · 3 comments
Open

Surprising default combining rule in association term #258

ianhbell opened this issue Feb 23, 2024 · 3 comments

Comments

@ianhbell
Copy link
Contributor

After quite some investigation my colleague and I figured out what was going on in the association implementation in Clapeyron and we were quite surprised to find that the default combining rule is no(!) combining rule. From my side, it seems like CR1 should probably be the default. I figured out eventually how to make the combining rule be CR1, which is not terribly well documented.

using Clapeyron
options = AssocOptions(combining=:cr1)
model = Clapeyron.CPA(["Ethanol", "Water"], assoc_options=options)
@pw0908
Copy link
Member

pw0908 commented Feb 23, 2024

On the documentation, admittedly, it's not in the most obvious place, but we did document the fact that we do have no combining rule is the default: https://clapeyronthermo.github.io/Clapeyron.jl/dev/api/parameters/#Clapeyron.AssocOptions.

As for what the default should be, the difficulty is that did SAFT equations use different combining rules (I believe that SAFT-VR Mie uses Elliott whereas CPA using CR1. When it comes to specifying association parameters though, I do believe that it is better to leave the combining rule off by default as, when you get to SAFT-VR Mie / gamma Mie where you have H, e1, e2 and a1 association sites where you don't necessarily expect cross-association between everything, it's safer to leave it up to the user to specify.

I will add some docs on that in the documentation that I'm updating.

@pw0908
Copy link
Member

pw0908 commented Feb 23, 2024

Happy to report the updated docs do explicitly highlight how the association interactions can be adjusted: https://github.com/ClapeyronThermo/Clapeyron.jl/blob/docs-update/docs/src/tutorials/basics_model_construction.md

@ianhbell
Copy link
Contributor Author

If you explicitly select CPA/sCPA, I feel like the default sent to the association term should be CR1. Or at least, it should be clearly documented in the model documentation for all the relevant models that the combining rule is not set at all. I see your point about the nuances of how the association term gets used in other places, and sympathize. I'm struggling with the same thing in teqp now.

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

No branches or pull requests

2 participants