Skip to content

Fix incorrect units for BSE-calculated spectra - #335

Open
ndaelman-hu wants to merge 1 commit into
developfrom
fix/spectra-dimensionless-units
Open

Fix incorrect units for BSE-calculated spectra#335
ndaelman-hu wants to merge 1 commit into
developfrom
fix/spectra-dimensionless-units

Conversation

@ndaelman-hu

Copy link
Copy Markdown
Contributor

Summary

Both Exciting and OCEAN parsers were incorrectly setting intensities_units = 'F/m' for BSE-calculated absorption spectra. These parsers actually calculate the macroscopic dielectric function ε(ω), which is dimensionless.

Background

The confusion arose from conflating two different quantities:

  • Dielectric function/constant (dimensionless) - what BSE calculates
  • Absolute permittivity (F/m units) - different quantity

Changes

Exciting parser (electronicparsers/exciting/parser.py:2333)

sec_spectra.intensities = data[2]  # Imaginary part of dielectric function  
sec_spectra.intensities_units = 'dimensionless'

OCEAN parser (electronicparsers/ocean/parser.py:239)

sec_spectra.intensities = data_spct[:, 2]  # BSE absorption/emission spectra (dielectric function)
sec_spectra.intensities_units = 'dimensionless'

Testing

  • All Exciting parser tests pass (8 passed, 1 skipped)
  • All OCEAN parser tests pass (1 passed)

References

Both Exciting and OCEAN parsers were setting `intensities_units = 'F/m'`
for BSE-calculated absorption spectra. However, these parsers calculate
the macroscopic dielectric function ε(ω), which is dimensionless.

The confusion arose from conflating:
- Dielectric function/constant (dimensionless) - what BSE calculates
- Absolute permittivity (F/m units) - different quantity

Changes:
- Exciting: Set `intensities_units = 'dimensionless'` for epsilon spectra
- OCEAN: Set `intensities_units = 'dimensionless'` for absorption spectra
- Added comments clarifying the physical quantity being computed

Fixes #334
Related: nomad-coe/nomad-schema-plugin-run#24
@ndaelman-hu

Copy link
Copy Markdown
Contributor Author

Breaking Change Warning

Changing `intensities_units` from `'F/m'` to `'dimensionless'` will affect all existing Exciting and OCEAN archives. This drops backwards read compatibility.

Recommendation: This change should be applied when we do a full reprocessing of Exciting and OCEAN archives to ensure consistency across the database. @ladinesa @JFRudzinski

@coveralls

Copy link
Copy Markdown

Coverage Report for CI Build 26090687936

Coverage increased (+0.04%) to 92.881%

Details

  • Coverage increased (+0.04%) from the base build.
  • Patch coverage: 4 uncovered changes across 2 files (0 of 4 lines covered, 0.0%).
  • 36 coverage regressions across 2 files.

Uncovered Changes

File Changed Covered %
electronicparsers/exciting/parser.py 2 0 0.0%
electronicparsers/ocean/parser.py 2 0 0.0%

Coverage Regressions

36 previously-covered lines in 2 files lost coverage.

File Lines Losing Coverage Coverage
electronicparsers/exciting/parser.py 23 71.0%
electronicparsers/exciting/metainfo/exciting.py 13 90.74%

Coverage Stats

Coverage Status
Relevant Lines: 38908
Covered Lines: 36138
Line Coverage: 92.88%
Coverage Strength: 0.93 hits per line

💛 - Coveralls

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.

Exciting parser sets incorrect units for dielectric function imaginary part

2 participants