Re-enable FHI-aims total energy mapping

[ndaelman-hu]

Purpose

Re-enables the total_energies mapping for FHI-aims parser that was disabled in commit 0e00561 (PR #150, March 2026). The parser's get_energies() method has remained functional - only the schema mapping annotation was commented out.

Scope

Included

• Re-enable total_energies mapping annotation in fhiaims.py
• Add comprehensive test coverage for energy extraction
• Validate energy values and contributions structure

Out of scope

• XC functional mappings (also commented out in Schema update convergence targets #150, but not needed for this fix)
• Duplicate annotation cleanup (lines 138-157 have redundant force/eigenvalue annotations)
• Integration with PR Migration of fundamental system and electronic properties for normalization to results #170 (large-scale electronic properties migration)

Context & Links

Why it was disabled: During the convergence targets migration (#150), the energy mapping was intentionally commented out using the old m_annotations.setdefault() syntax. The commit message stated: "Future migration to dict-based m_def mapping annotations should be tracked separately (see PR #162 for reference)."

Related PRs:

• Schema update convergence targets #150 - Original PR that commented out energy mapping (merged March 2026)
• Migration of fundamental system and electronic properties for normalization to results #170 - Large electronic properties migration (open, includes FHI-aims + 17 other parsers)
• Backup/test data normalization fhiaims 2026 05 12 #176 - Backup branch of FHI-aims changes from Migration of fundamental system and electronic properties for normalization to results #170
• Add FHI-aims SCF convergence criteria extraction #177 - SCF convergence criteria extraction (merged May 2026)

Investigation notes: See .investigation-notes.md in branch (not committed) for detailed impact analysis.

Reviewer Notes

Key changes:

1. Replace commented m_annotations.setdefault() with active add_mapping_annotation() for consistency with other parsers (VASP, GPAW, QE all use this pattern)
2. New test test_total_energies() validates:
   • Energy extraction across all 5 geometry optimization steps
   • Correct energy value (-15696.12 eV for first step)
   • Energy contributions/components structure
   • Pint Quantity handling

Risk assessment: LOW

• Parser method get_energies() already works (parser.py:289-301)
• MD parser utils are defensive (check if total_energies is empty before appending)
• No existing tests affected (no prior energy tests)
• Follows established pattern from 10+ other parsers

Status

• Ready for review

Breaking Changes

• None

Dependencies / Blockers

• None
• Independent of PR Migration of fundamental system and electronic properties for normalization to results #170 (that PR re-enables energies too, but for all parsers)
• Can be merged before or after Migration of fundamental system and electronic properties for normalization to results #170 without conflict

Testing / Validation

• Unit tests: Added test_total_energies() with comprehensive validation
• All FHI-aims tests pass (7/7)
• Energy values match test data analysis
• Energy contributions properly structured

[coveralls]

Coverage Report for CI Build 26894435401

Coverage increased (+0.2%) to 74.352%

Details

• Coverage increased (+0.2%) from the base build.
• Patch coverage: 26 of 26 lines across 2 files are fully covered (100%).
• No coverage regressions found.

Uncovered Changes

No uncovered changes found.

Coverage Regressions

No coverage regressions found.

---

Coverage Stats

Relevant Lines:	10371
Covered Lines:	7711
Line Coverage:	74.35%
Coverage Strength:	0.74 hits per line

---

💛 - Coveralls

[Copilot]

Pull request overview

This PR re-enables the FHI-aims total_energies schema mapping by restoring an active mapping annotation (using add_mapping_annotation) and adds unit tests to verify total energy extraction and contribution structure for a geometry optimization run.

Changes:

• Re-enabled outputs.Outputs.total_energies mapping annotation in the FHI-aims schema package.
• Added a new test_total_energies() to validate extracted total energies across all geometry optimization steps, including units and contribution presence.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File	Description
src/nomad_simulation_parsers/schema_packages/fhiaims.py	Restores total_energies mapping annotation using add_mapping_annotation under the FHI-aims text mapping key.
tests/parsers/test_fhiaims_parser.py	Adds coverage ensuring total_energies are populated for each output step and validates the first-step reference value and contributions.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[EBB2675]

Aren't is not None assertions on total_energies and contributions vacuous?

afaik, SubSection with repeats=True never returns None on access, but it returns an empty MSubSectionList

[EBB2675]

(source: metainfo.py:3487-3494)

def __get__(self, obj, cls=None):
    if obj is None:
        return self

    if self.repeats:
        return obj.__dict__.setdefault(self.name, MSubSectionList(obj, self))
    # non-repeating:
    return obj.__dict__.get(self.name, None)

[EBB2675]

LGTM, one nit on dead is not None assertions in the tests