search

zerothi / sisl

Electronic structure Python package for post analysis and large scale tight-binding DFT/NEGF calculations
https://zerothi.github.io/sisl
Mozilla Public License 2.0
182 stars 58 forks source link

Reading spin densities from CHGCAR #754

Closed tfrederiksen closed 5 months ago

tfrederiksen commented 5 months ago

The spin densities were not correctly read from CHGCAR, because after the augmentation occupancies an additional block of values (apparently one per atom) is found on file.

I could not determine what this extra block refers to, as it is not explicitly mentioned in the documentation from the VASP forum, which just mentions:

For magnetic calculations, the CHGCAR file contains additional data blocks for the magnetization. In particular, for spin-polarized calculations (ISPIN=2), the first set contains the total charge density (spin up + spin down) and the second one is the magnetization density (spin up - spin down):

  • Structure
  • FFT-grid dimensions
  • Charge density times FFT-grid volume (spin up + spin down)
  • Augmentation occupancies
  • FFT-grid dimensions
  • Magnetization density (spin up - spin down)
  • Augmentation occupancies

This PR resolves this issue and adds some further checks.

codecov[bot] commented 5 months ago

Codecov Report

Attention: Patch coverage is 96.42857% with 1 lines in your changes are missing coverage. Please review.

Project coverage is 86.78%. Comparing base (71360c5) to head (ff56451).

:exclamation: Current head ff56451 differs from pull request most recent head ed7d651. Consider uploading reports for the commit ed7d651 to get more accurate results

Files Patch % Lines
src/sisl/io/vasp/chg.py 75.00% 1 Missing :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #754 +/- ## ======================================= Coverage 86.77% 86.78% ======================================= Files 410 410 Lines 51785 51811 +26 ======================================= + Hits 44939 44965 +26 Misses 6846 6846 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

tfrederiksen commented 5 months ago

Could we add a link to the documentation or something, so it is clear what the indices of grid values refers too?

Where would you add a link? The docstring already describes the index and spin arguments, no?

zerothi commented 5 months ago

Could we add a link to the documentation or something, so it is clear what the indices of grid values refers too?

Where would you add a link? The docstring already describes the index and spin arguments, no?

It seems to me that line 35 is then wrong, no?

tfrederiksen commented 5 months ago

It seems to me that line 35 is then wrong, no?

Oh, I see. Yes, indeed index=1 would give the spin density and not the down-component as currently stated. By the way, why two keywords index/spin for the same thing? Wouldn't it not be better to have just one?

zerothi commented 5 months ago

It seems to me that line 35 is then wrong, no?

Oh, I see. Yes, indeed index=1 would give the spin density and not the down-component as currently stated. By the way, why two keywords index/spin for the same thing? Wouldn't it not be better to have just one?

Yes, currently this is a left-over from the siesta output. A think we could investigate is whether index should be used for explicit retrieval (no manipulation), and spin="x" could be used for explicit spin-configuration extraction, or spin=Spin.X.

Probably that should also be stream-lined.

tfrederiksen commented 5 months ago

Yes, currently this is a left-over from the siesta output. A think we could investigate is whether index should be used for explicit retrieval (no manipulation), and spin="x" could be used for explicit spin-configuration extraction, or spin=Spin.X.

Probably that should also be stream-lined.

I've now updated the docstring and included some examples. I think the stream-lining could be a future PR.