Codecov Report

Attention: Patch coverage is 77.77778% with 10 lines in your changes missing coverage. Please review.

Project coverage is 67.60%. Comparing base (36af095) to head (bfb64f3).

@@            Coverage Diff             @@
##             main    #1041      +/-   ##
==========================================
- Coverage   67.60%   67.60%   -0.01%     
==========================================
  Files         112      112              
  Lines        6582     6599      +17     
==========================================
+ Hits         4450     4461      +11     
- Misses       2132     2138       +6     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@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.
tot_magnetization: Specifies the imposed difference between spin-up and spin-down electrons.

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!

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 magnetic_moment is selected:

If a nonzero ground-state magnetization is expected, you must assign a nonzero value to at least one atomic type. For an antiferromagnetic state, define two distinct atomic species representing the sublattices of the same atomic type.

When Tot_magnetization is selected the help text should say:

The difference between majority spin charge and minority spin charge. Used to specify the desired total electronic magnetization.

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 magnetic_moment is selected:

If a nonzero ground-state magnetization is expected, you must assign a nonzero value to at least one atomic type. For an antiferromagnetic state, define two distinct atomic species representing the sublattices of the same atomic type.

When Tot_magnetization is selected the help text should say:

The difference between majority spin charge and minority spin charge. Used to specify the desired total electronic magnetization.

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 starting_magnetization. We use it in references to the parameters dictionary throughout. I could change this, but I'm concerned at its affect on backwards compatibility. Not sure you'd be able to load older calculations. Need to test. That said, the text of the toggle button is already changed to "Magnetic moments", as this does not impact the code.