-
Notifications
You must be signed in to change notification settings - Fork 15
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
Use plugin utility to set default starting magnetization #1041
base: main
Are you sure you want to change the base?
Use plugin utility to set default starting magnetization #1041
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1041 +/- ##
==========================================
+ Coverage 67.86% 67.94% +0.07%
==========================================
Files 112 112
Lines 6679 6726 +47
==========================================
+ Hits 4533 4570 +37
- Misses 2146 2156 +10
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
@giovannipizzi please confirm the screenshots in the description follow what you had in mind 🙏 |
Looks very good! We can refine in the future if needed, but already now is great |
@AndresOrtegaGuerrero haven't worked much with magnetic calculations. Was wondering how to handle total magnetization now. If starting magnetization is provided, does QE computes total magnetization internally, or do we need to explicitly provided it as well? If so, is it simply the sum of the starting magnetic moments? Note that at the moment, we don't have this implemented in the app. Total magnetization does not observe starting magnetic moments. |
In the starting_magnetization widget, we're actually passing the initial_magnetic_moments. (Perhaps we should consider renaming the widget for clarity.) These values are then passed to the workchain in the qe-plugin, which generates the starting_magnetization based on the provided magnetic moments. builder = PwBaseWorkChain.get_builder_from_protocol(
structure=new_structure,
code=pw_code,
protocol='fast',
spin_type=SpinType.COLLINEAR,
initial_magnetic_moments=results['magnetic_moments']
) The values you set in the starting_magnetization widget are treated as magnetic_moments, which are then used by the get_starting_magnetization function to calculate the starting_magnetization values in the input. You can verify this behavior with a test. It’s important to understand the distinction between starting_magnetization and total_magnetization. starting_magnetization: Defined per atomic kind, QE will calculate and output the final magnetization after convergence. For more details, refer to the QE documentation: Quantum ESPRESSO Input Description. |
One possible solution for #982 could be to screen the dictionary from the starting_magnetization widget. If all values are 0.0, we should pass None. This would allow the PwBaseWorkChain to compute the starting_magnetization values (though this should be tested). If any value is different from 0.0, we pass the full dictionary instead. In this case, renaming the widget to initial_magnetic_moment might make the purpose clearer. |
@AndresOrtegaGuerrero will you be available on Monday to discuss? I have many questions! |
f8e9aaa
to
4087452
Compare
So we should rename the widget to magnetic moment, since that is the information we give the PwBaseWorkChain from Qe-plugin. I think we should have a text below the magnetization title that helps the user.
When Tot_magnetization is selected the help text should say:
|
4087452
to
c4b23a9
Compare
Thanks @AndresOrtegaGuerrero! I'm okay with the text. I would maybe add a line emphasizing that the moments are initial, to be relaxed by the calculation. Maybe also @giovannipizzi can review? Regarding renaming of the widget, one question I had is how to handle remaining references to |
c4b23a9
to
bfb64f3
Compare
bfb64f3
to
8da6ddc
Compare
The units of the widget are Bohr magneton (meaning the number of unpaired electrons). QE plugin takes care of converting this value in the units expected in pw.x input (namely starting_magnetization) a value between -1 and 1 (in pw.x not in the App) where 1 means that all electrons available for the selected species will be alpha electron. Si - Starting magnetization widget = 1 pw.x input |
8da6ddc
to
5984ed4
Compare
@AndresOrtegaGuerrero @cpignedoli in this PR, I'm using the plugin's def get_starting_magnetization(
structure: StructureData,
pseudo_family: PseudoPotentialFamily,
initial_magnetic_moments: Optional[dict] = None
) -> dict:
"""Return the dictionary with starting magnetization for each kind in the structure.
:param structure: the structure.
:param pseudo_family: pseudopotential family.
:param initial_magnetic_moments: dictionary mapping each kind in the structure to its magnetic moment.
:returns: dictionary of starting magnetizations.
"""
if initial_magnetic_moments is not None:
nkinds = len(structure.kinds)
if sorted(initial_magnetic_moments.keys()) != sorted(structure.get_kind_names()):
raise ValueError(f'`initial_magnetic_moments` needs one value for each of the {nkinds} kinds.')
return {
kind.name: initial_magnetic_moments[kind.name] / pseudo_family.get_pseudo(element=kind.symbol).z_valence
for kind in structure.kinds
}
magnetic_parameters = get_magnetization_parameters()
starting_magnetization = {}
for kind in structure.kinds:
magnetic_moment = magnetic_parameters[kind.symbol]['magmom']
if magnetic_moment == 0:
magnetization = magnetic_parameters['default_magnetization']
else:
z_valence = pseudo_family.get_pseudo(element=kind.symbol).z_valence
magnetization = magnetic_moment / float(z_valence)
starting_magnetization[kind.name] = magnetization
return starting_magnetization to obtain the starting magnetization, then scale it by the valence provided in the selected pseudo to obtain the moments. Now, Carlo was wondering why 0.4 for Si. Note that def get_magnetization_parameters() -> dict:
"""Return the mapping of suggested initial magnetic moments for each element.
:returns: the magnetization parameters.
"""
with (pathlib.Path(__file__).resolve().parent / 'magnetization.yaml').open() as handle:
return yaml.safe_load(handle) In "magnetization.yaml", Si's Is this correct logic? |
Thanks a lot, @edan-bainglass for digging this out. From my use cases this is not ideal. If I am computing Si and for some reason I switch on magnetism I still want to have 0 as suggested starting magnetization. Moreover, 0.1 converted to 0.4 is additionally not clear to me: So in the case of Si or C, if I set 1 Bohr magneton in the QE app GUI I should see 0.25 in pw.inp |
This is a bit difficult to do. I could apply logic to intercept magnetization of 0.1 and zero them out, but that would be assuming that no magmom divided by its pseudo-dependent Z valence value would somehow also hit 0.1, ending up with zero erroneously! Need to think about this some more.
This is all correct and currently implemented. If magmom = 5 in the yaml file, |
Okay, I can solve this. Give me a few minutes 🙏 |
Automatic defaults 👍 Note the correct 5 for Co and 0s for the rest. @cpignedoli expected? |
But this I think leads to the same issue, no? If I choose bulk silicon and select magnetism, this "solution" will zero out silicon, thus leading to a non-magnetic calculation! @giovannipizzi @cpignedoli what should we do here? This solution is correct perhaps for the expert, but for beginners, this might be confusing. |
Thanks a lot @edan-bainglass This reflects what I have in mind. We should warn the user that |
In the function that generates the initial moments, we indeed put 0.1 to ensure we break the "symmetry" and possibly detect any magnetism. If none should be there, the scf should bring it back to zero. This was discussed for quite some time, pinging @mbercx but also @Minotakm @t-reents For this release, if @cpignedoli is OK, I'd keep the logic in the function (putting some small nonzero also for Si) and we rediscuss. @edan-bainglass this might need a slight change of the explanatory sentence discussed yesterday. Also as a note, we had a report that setting too high initial moments might make convergence harder and we'll investigate this in the future, so better to discuss this as part of aiida-qe to avoid having two different approaches |
To be clear, a warning is already in place, pushing users to set the magnetic configuration in the advanced settings. Are you suggesting something in addition to this warning? Note also that once they arrive at the widget in the advanced settings, a help message does instruct the user to set nonzero values to reach a nonzero ground-state magnetization. So I think we're covered? |
sorry @edan-bainglass @giovannipizzi I was not aware of the previous discussions, if the 0.1 was put in place for a good reason, then let's keep it together with the appropriate warnings, I am fine with it. What is important is the consistency with units, the .yaml has the same units as we have in the QE app GUI so we are OK. |
Yes, as long as I apply the rescaling in general, the units are correct. Note that this means for those species of zero magmom, 0.1 magnetization will be rescaled into magmom depending on the Z valence of the element defined in the pseudo. This is how we end up with magmom 0.4 for Si. But it will return back to 0.1 magnetization in the PW input file. As long as we are okay with this, I can implement. |
This PR uses the QE plugin
get_starting_magenetization
utility function to set default initial magnetic moments for the structure when magnetism is turned on. The defaults are set to the default magmom provided by the utility scaled by the Z valence value for the element defined in the selected pseudopotential. Note that for those elements of a zero magmom default, a general default magnetization of 0.1 is returned.If multiple tags are provided for the same species, a warning is displayed providing some instructions for antiferromagnetic configurations.
Closes #982