-
Notifications
You must be signed in to change notification settings - Fork 13
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
Add association term of Dufal #131
Comments
@longemen3000 I implemented this on the Dufal branch, would you care to take a look? The code is a bit messy because I am trying to keep the compilation time to a reasonable level which makes it not possible to use the nice C++ techniques I have been using elsewhere (one must keep the number of template instantiations to a minimum, which implies the use of runtime branching rather than compiler branching). I have the experience that the code with this model is about 10x slower than the normal association model for pure water. Is this your experience? |
on the current Clapeyron impl, there was a bug that makes that specific impl slower (some julia specifics). after fixing that, i got the following timings:
so, around 2x. (i did not take care of using an appropiate reduction technique to skip the repeated powers, but that can be done on both mie impls). i also checked with the dufal paper, and it seems that i can reproduce the data of association non bonded fractions in table 4:
|
After fixing the erratum in Dufal that they mention in their corrigendum, I get something more reasonable:
The timing is for the entire construction of the matrix, which is incompletely optimized in my case since all four entries of the 4x4 matrix are identical and I do the same calculation 4 times (doh). My matrices are:
for the normal one and
for Dufal. I don't know why there is a small but nonzero difference. Maybe it is about the value of R used? I am surprised that your "normal" implementation is so slow, since almost nothing is happening in the evaluation function, at least compared with Dufal, and that looks like an opportunity for further optimization on the Clapeyron side. |
For combination with SAFT-VR-Mie. From https://doi.org/10.1080/00268976.2015.1029027
The text was updated successfully, but these errors were encountered: