-
Notifications
You must be signed in to change notification settings - Fork 12
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
Feature minimise #163
Merged
Merged
Feature minimise #163
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…ixed a bug in the new functionality where a default lever for a force was not being picked up (i.e. `ghost/ghost::*` was not taking precedence over, e.g. `*::charge`) I've updated the unit tests to catch these edge cases
…times to find a stable starting dynamics structure. This solves the weird aladip structure problem, but runs minimisation too long for tyk2 (getting confused when constraints are re-applied)
…ore robust with the way constraints are handled.
This includes vectorising for Apple Silicon and adding more tests for convergence so that we can have more confidence that the structures output are sensible. Also made sure that optimised compilation (-O3) is used for all of the plugins (SireOpenMM, SireGemmi and SireRDKit). They were previously compiled with wrapper options (e.g. -Os). Minimisation now gives better progress updates, using a progress bar to show progress towards the maximum number of iterations. This has been reduced to 1500 by default. Also, if the minimisation fails to create a structure that obeys constraints on the first pass, then the minimisation is repeated, with the maximum number of iterations reset. If it fails again, then this structure, with constraints re-applied, is returned.
Ran the wrapper generation again, which added docs to PerturbableOpenMMMolecule Moved the optimised cxxflags to a higher-level CMakeLists.txt and added this properly, rather than duplicating flags. Will see if this fixes the Linux compile error. (will check locally, hence skipping CI) [ci skip]
…ebugging info to minimisation output for problematic cases
minimisation to the user via arguments with sensible defaults to the minimisation functions. The documentation of the functions explains how the ratchetting algorithm works. Also updated the progress bar so that you can see the number of repeats and number of ratchets (e.g. 1.5 would be the sixth ratchet of the first run) Also fixed the double-header output for the create_wrapper script and regenerated the SireOpenMM wrappers. This also now automatically finds the python header files.
[ci skip]
…things. You can print the log using the `.get_log()` function of `Minimisation`, e.g. ``` m = mols.minimisation().run() print(m.get_log()) ```
This now will do a hard reset and repeat once only, if a NaN or other extremely broken state is detected. It will also reset the minimizer parameters if they get into a weird state. This still doesn't successfully minimise everything - but it isn't bad :-)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR brings back the improved minimiser code (originally in #162). This makes the minimiser even more robust, and includes better detection of NaNs, and tuning from the return value of the underlying algorithm.
devel
into this branch before issuing this pull request (e.g. by runninggit pull origin devel
): [y]Suggested reviewers:
@lohedges
Any additional context of information?
It's worth testing again as I have updated the default parameters. They are now all exposed via the
Minimisation.run()
function.