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

Implementation of polar SAFT-VR Mie #302

Merged
merged 15 commits into from
Oct 16, 2024

Conversation

sonjaamsmith
Copy link
Contributor

Addition of the Gross and Vrabec dipole (2006), quadrupole (2005), and dipole-quadrupole (2008) terms to the SAFT-VR Mie framework as implemented by Cripwell et al. (2018) and Smith et al. (2020).

@longemen3000
Copy link
Member

longemen3000 commented Oct 15, 2024

Nice!, some things to do before merging:

  • There is a bunch of files named singledata_EOS for UNIFAC, PR, and PCSAFT, that are unused, maybe delete those?
  • can you add a test checking if the EoS gives a reasonable result?

@pw0908 any comments?

@pw0908
Copy link
Member

pw0908 commented Oct 15, 2024

@longemen3000 One thing @sonjaamsmith pointed out is that we use the Hudson-McCoubrey combining rule instead of the sqrt version in SAFT-VR Mie. Do you remember why we did this? I seem to recall we had a conversation about this a long time ago but can't remember why we didn't change it.

@pw0908
Copy link
Member

pw0908 commented Oct 15, 2024

Found the relevant issue; it's an outstanding mistake. We switched it for SAFT-gamma Mie but not VR Mie

@sonjaamsmith
Copy link
Contributor Author

I removed those unused files and added a test.

@longemen3000 longemen3000 merged commit 218e8ef into ClapeyronThermo:master Oct 16, 2024
7 of 10 checks passed
pw0908 added a commit that referenced this pull request Oct 16, 2024
@pw0908
Copy link
Member

pw0908 commented Oct 16, 2024

Was about to commit the fix to SAFT-VR mie but it's now been added to master

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.

3 participants