Fix overriding magmoms in update_user_incar_settings()

[janosh]

Closes #374.

This PR gives magmoms passed to _get_magmoms() precedence over magmoms already in the structure.

It also adds a test for _get_magmoms().

[utf]

Thanks @janosh. I think this might need a bit more thought.

Previously the magmom precedence was:

• magmoms in input struct
• spins in input struct
• user incar settings
• config dict
• set all magmoms to 0.6

The actual precedence for magmoms should be:

• user incar settings
• magmoms in input struct
• spins in input struct
• config dict
• set all magmoms to 0.6

However, this PR has the order

• user incar settings
• config dict
• magmoms in input struct
• spins in input struct
• set all magmoms to 0.6

That's because the config dict is updated with user_incar_settings, and the magmom settings are then passed to _get_magmoms. E.g., see here:

atomate2/src/atomate2/vasp/sets/base.py

Lines 611 to 614 in 6c541cd

	incar_settings = dict(self.config_dict["INCAR"])
	# apply user incar settings to SETTINGS not to INCAR
	_apply_incar_updates(incar_settings, self.user_incar_settings)

And here:

atomate2/src/atomate2/vasp/sets/base.py

Line 620 in 6c541cd

incar[k] = _get_magmoms(v, structure)

Solution

To get the right precedence, one thing we could do is:

1. Before

   atomate2/src/atomate2/vasp/sets/base.py

   Line 614 in 6c541cd

   _apply_incar_updates(incar_settings, self.user_incar_settings)

   copy the config dict magmoms to a separate variable.
2. Update the _get_magmom function to also accept the original config dict, and update the logic with the correct precedence.

It would also be nice to update the precedence of how magmoms are set in both the docstring get_magmom and as a note in the docstring of the VaspInputGenerator class.

[janosh]

@utf Thanks for the quick feedback. By config dict magmoms, you mean incar_magmoms = incar_settings.get("MAGMOM", {})?

[utf]

Yes, although this should be before the user_incar_settings are applied. E.g.,

incar_settings = dict(self.config_dict["INCAR"]) 
base_magmoms = incar_settings.get("MAGMOM", {})

# apply user incar settings to SETTINGS not to INCAR 
_apply_incar_updates(incar_settings, self.user_incar_settings) 

[janosh]

Lunch now! I'll do the docs update later.

[codecov]

Codecov Report

Merging #375 (1244074) into main (2f54128) will increase coverage by 0.10%.
The diff coverage is 100.00%.

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #375      +/-   ##
==========================================
+ Coverage   64.74%   64.85%   +0.10%     
==========================================
  Files          74       74              
  Lines        7157     7161       +4     
  Branches      938      939       +1     
==========================================
+ Hits         4634     4644      +10     
+ Misses       2224     2220       -4     
+ Partials      299      297       -2     

Impacted Files	Coverage Δ
src/atomate2/common/schemas/defects.py	87.91% <ø> (ø)
src/atomate2/vasp/sets/base.py	69.23% <100.00%> (+1.93%)	⬆️