remove annotations fixes#133
Open
JFRudzinski wants to merge 1 commit into
Open
Conversation
Collaborator
Author
|
@ndaelman-hu what I don't know is if the solution in nomad-FAIR is only arising because of this fix and/or if there is a different way to solve that issue directly in the parser |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
VASP OUTCAR Parser Fix
Symptoms
tests/parsers/test_vasp_parser.py::test_outcarpassed when executed alone but failed (AssertionError: No model_method in simulation) once the full parser suite ran.Root Cause
general.Simulation.model_methodsubsection, so the OUTCAR mapping could not materialize the parent before fillingmodel_method.DFT.exciting,FHI-aims,H5MD) calledremove_mapping_annotationswith no filter, which wiped all mapping keys from the sharedgeneral.Simulationdefinition after their runs, erasing the VASP mappings even after they were added.Fix
src/nomad_simulation_parsers/schema_packages/vasp.py:remove_mapping_annotationsto accept anannotation_keysfilter and updated theexciting,FHI-aims, andH5MDparsers to drop only their own mapping keys. This keeps their cleanup behavior without stripping VASP’sxml/outcarmappings.Verification
Both commands now pass;
test_outcarsucceeds regardless of suite order.Notes
IndexError: pop index out of rangeinsideSimulation.model_systemremoval) were ultimately due to the shared mapper cache leaking data between parsers. The FIX lives innomad-FAIR; without it you would have to run QE tests in isolation because there’s no robust way to prevent the cache from reusing ABINIT’s data mid-suite.