-
Notifications
You must be signed in to change notification settings - Fork 189
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
Update the constant-pH tutorial #4262
Conversation
Check out this pull request on See visual diffs & provide feedback on Jupyter Notebooks. Powered by ReviewNB |
Some interesting observations on speedup:
If the non_interacting_type has such an effect on the speed, I think it is desired to give it a fixed and low value, or to set dynamically to the lowest possible value that is available. Maybe we should consider using some of these tricks also in the tests of the RxMC and cpH algorithms. It is not important to get perfectly correct physics but to ensure that we can still reproduce the titration curve of an interacting system if we change something in the algorithm. What do you think of that? @jngrad @RudolfWeeber |
Good idea, I can have a look at this later this week to speed up the testsuite, and mention it in the docs too. A couple of tests did not pass CI. Let us know when you are ready with the PR and we'll review it. |
TODO: before this PR is merged, we should change the initial set of parameters back to the ideal system and clear the cells with solutions of the excercises. |
prob_reaction, more on Bjerrum and thermodynamics. Use easier-to-read nested cycles for non-bonded interactions between all particle pairs.
Use Debye-Hückel electrostatics, specify a non_interacting_type and apply other tricks to speed-up the calculation.
b8013b6
to
e0db850
Compare
Properly stylize "pH" and "pKa" throughout the text. Fix plot labels, typos and formatting. Address TODOs. Make citations link to the bibliographic entries and cleanup links to the user guide. Run the tutorial without electrostatics to reproduce the ideal case.
e0db850
to
b0e3b57
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the improvements!
Fixes #3941
Description of changes: