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

[WIP] Heisenberg #1525

Merged
merged 25 commits into from
Jul 17, 2019
Merged

[WIP] Heisenberg #1525

merged 25 commits into from
Jul 17, 2019

Conversation

ncfrey
Copy link
Contributor

@ncfrey ncfrey commented Jul 15, 2019

Summary

@mkhorton

New functionality to take low energy magnetic orderings, map them to a Heisenberg model, compute exchange parameters, and perform Monte Carlo with the VAMPIRE package to calculate critical temperatures.

  • Heisenberg model mapping of magnetic materials for calculating exchange parameters.
  • Wrapper for VAMPIRE atomistic simulations of magnetic materials package.

Additional dependencies introduced (if any)

  • Vampire5 command line tool for atomistic simulations of magnetic materials.

TODO (if any)

  • Added sophistication to screening/sanitization of input low energy ordering data.
  • Cleanup VAMPIRE files after execution.

@mkhorton
Copy link
Member

Thanks @ncfrey !

@shyuep
Copy link
Member

shyuep commented Jul 15, 2019

I propose that you name the pymatgen wrappers around vampire "lestat". :-)

@mkhorton
Copy link
Member

Looks pretty good, some tests need to be updated (e.g. see test_round_magmoms in CollinearMagneticStructureAnalyzerTest), a few minor PEP8 violations (mostly line length), HeisenbergScreener class definition has extraneous parentheses etc. I haven't had a look at test coverage yet. Overall seems v high quality though + almost ready to merge. Thanks for the good code comments, made reviewing it easy!

@ncfrey
Copy link
Contributor Author

ncfrey commented Jul 16, 2019

Thanks for the review Matt! Working on those fixes now.

@mkhorton
Copy link
Member

Find it really weird that ReactionDiagramTest failed here ... there's nothing in this PR that should affect that.

@mkhorton mkhorton merged commit ca6a611 into materialsproject:master Jul 17, 2019
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