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.

Sign up for GitHub

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

Jump to bottom

Monte Carlo methods based on AdvancedMH.jl do not provide expected results. #320

Closed
Alexsp32 opened this issue Dec 1, 2023 · 1 comment · Fixed by #325
Closed

Monte Carlo methods based on AdvancedMH.jl do not provide expected results. #320

Alexsp32 opened this issue Dec 1, 2023 · 1 comment · Fixed by #325
Assignees
@Alexsp32
Labels
bug Something isn't working
Milestone
End of 2023 update

Comments

@Alexsp32
Copy link
Member

Alexsp32 commented Dec 1, 2023 •
edited
Loading

Expected behaviour: For small step sizes and move ratios, Monte Carlo sampling should yield geometries close to the starting geometry with minimal displacements. This is with a high-dimensional model of 3*56 degrees of freedom.

Observed behaviour: Structures are drastically altered with all atoms at unrealistically close distances and positions within [0,1] for every atom and degree of freedom. Displacements from these "exploded" geometries are minimal and do not obey unit cell constraints.

This appears to be particular to AdvancedMH v0.8, since I can't recreate the issue with AdvancedMH pinned to v0.7.4. I will update the issue once I find out more.
https://github.com/NQCD/NQCDynamics.jl/blame/0d22b506cc092ac88a2eba90aab587ac5c1f681a/Project.toml#L49

This also means that the automated tests for the MC wrappers to other packages are not thorough enough. It may be worth implementing a test case related to a real system.
@Alexsp32 Alexsp32 added the bug Something isn't working label Dec 1, 2023
@Alexsp32 Alexsp32 added this to the End of 2023 update milestone Dec 1, 2023
@Alexsp32 Alexsp32 self-assigned this Dec 1, 2023
Alexsp32 added a commit that referenced this issue Dec 4, 2023
@Alexsp32
Attempted fix for #320
f7d45ab 
AdvancedMH or one of its dependencies may have updated a keyword from "initial_config" to "initial_params".
This caused NQCD to not deliver initial positions to the sampling chain correctly.
@Alexsp32
Copy link
Member Author

Alexsp32 commented Dec 4, 2023

I might have the culprit - another dependency update: TuringLang/AdvancedHMC.jl#355

Seems like someone wanted to put in a warning about the breaking change, but their pull request was yanked 🙃

Alexsp32 added a commit that referenced this issue Dec 5, 2023
@Alexsp32
Drop compat for AdvancedMH.jl<0.8 for #320,#321
3e18b56
@Alexsp32 Alexsp32 mentioned this issue Dec 5, 2023
Closed
Alexsp32 added a commit that referenced this issue Dec 11, 2023
@Alexsp32
Attempted fix for #320
efef3db 
AdvancedMH or one of its dependencies may have updated a keyword from "initial_config" to "initial_params".
This caused NQCD to not deliver initial positions to the sampling chain correctly.
Alexsp32 added a commit that referenced this issue Dec 11, 2023
@Alexsp32
Drop compat for AdvancedMH.jl<0.8 for #320,#321
043da11
@Alexsp32 Alexsp32 mentioned this issue Dec 11, 2023
Merged
@Alexsp32 Alexsp32 linked a pull request Dec 11, 2023 that will close this issue
Update AdvancedMH.jl wrapper for Monte Carlo sampling. #325
Merged
Alexsp32 added a commit that referenced this issue Jan 25, 2024
@Alexsp32
Update AdvancedMH.jl wrapper for Monte Carlo sampling. (#325)
b6c2a73 
1. AdvancedMH or one of its dependencies may have updated a keyword from "initial_config" to "initial_params".
This caused NQCD to not deliver initial positions to the sampling chain correctly, leading to weird results for systems not centred around 0. 

2. Updated new keywords `movement_ratio` and `stop_ratio`, where `movement_ratio=1-stop_ratio` as the old `move_ratio` was confusing. Warning for deprecated kwargs was added and documentation updated with information about new keywords

* Drop compat for AdvancedMH.jl<0.8 for #320,#321

* Version bump for #325
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@Alexsp32 Alexsp32

Labels
bug Something isn't working
Projects
None yet
Milestone
End of 2023 update
Development

Successfully merging a pull request may close this issue.

Update AdvancedMH.jl wrapper for Monte Carlo sampling. NQCD/NQCDynamics.jl
1 participant
@Alexsp32