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

Fix for MagneticOrderingsWF static only #768

Merged
merged 9 commits into from
Jul 25, 2023

Conversation

fraricci
Copy link
Contributor

@fraricci fraricci commented Jan 9, 2023

I realized that the MagneticOrderingsWF did not work properly when static = True (static calculation only).
Here it is a fix that seems to work fine.

To note that I had to code explicitly "structure is not None" in the StaticFW.
I'm not sure why, probably it's due to the fact that the structure generated
in a MagneticOrderingsWF is a TransformedStructure obj and not a standard Structure obj.

I fixed also a bug in the calculation of the total magnetization per formula unit.

@fraricci fraricci changed the title Fix for MagneticWF static only Fix for MagneticOrderingsWF static only Jan 9, 2023
Copy link
Contributor

@Zhuoying Zhuoying left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for reporting this bug. The term num_formula_units needs to be corrected rather than the way to calculate total_magnetization_per_formula_unit. Could you do a quick fix and I will merge this PR after all tests pass.

@fraricci
Copy link
Contributor Author

Let me know if anything else is needed here.
I think the checks are failing for reasons unrelated to my commits.
Thanks

@Zhuoying
Copy link
Contributor

Hi @fraricci, thanks for the fix.
I recently merged some PRs to fix the previous circleci test failures (they should not be related to your PR).
Could you git pull these changes and retrigger the test? If the PR passes the pytest, I am merging it right now.
Thanks!

@Zhuoying Zhuoying merged commit f4060e5 into hackingmaterials:main Jul 25, 2023
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.

2 participants