From d4fec928aafafa95b756357c0fa2fe41aab62859 Mon Sep 17 00:00:00 2001 From: JosePizarro3 Date: Wed, 5 Jun 2024 10:44:52 +0200 Subject: [PATCH 01/12] Fixed outputs.py module after improved testing --- src/nomad_simulations/outputs.py | 63 ++++--- tests/conftest.py | 36 ++-- tests/test_outputs.py | 306 +++++++++++++++++++++++++++---- 3 files changed, 329 insertions(+), 76 deletions(-) diff --git a/src/nomad_simulations/outputs.py b/src/nomad_simulations/outputs.py index 42e533fe..07e26b40 100644 --- a/src/nomad_simulations/outputs.py +++ b/src/nomad_simulations/outputs.py @@ -17,7 +17,7 @@ # from structlog.stdlib import BoundLogger -from typing import Optional +from typing import Optional, List from nomad.datamodel.data import ArchiveSection from nomad.metainfo import Quantity, SubSection @@ -103,7 +103,9 @@ class Outputs(ArchiveSection): # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # - def extract_spin_polarized_property(self, property_name: str) -> list: + def extract_spin_polarized_property( + self, property_name: str + ) -> List[PhysicalProperty]: """ Extracts the spin-polarized properties if present from the property name and returns them as a list of two elements in which each element refers to each `spin_channel`. If the return list is empty, it means that the simulation is not @@ -113,7 +115,7 @@ def extract_spin_polarized_property(self, property_name: str) -> list: property_name (str): The name of the property to be extracted. Returns: - (list): The list of spin-polarized properties. + (List[PhysicalProperty]): The list of spin-polarized properties. """ spin_polarized_properties = [] properties = getattr(self, property_name) @@ -165,27 +167,36 @@ class SCFOutputs(Outputs): def get_last_scf_steps_value( self, - scf_last_steps: list, + scf_last_steps: List[Outputs], property_name: str, i_property: int, - scf_parameters: SelfConsistency, + scf_parameters: Optional[SelfConsistency], logger: BoundLogger, ) -> Optional[list]: """ Get the last two SCF values' magnitudes of a physical property and appends then in a list. Args: - scf_last_steps (list): The list of the last two SCF steps. + scf_last_steps (List[Outputs]): The list of SCF steps. This must be of length 2 in order to the method to work. property_name (str): The name of the physical property. i_property (int): The index of the physical property. + scf_parameters (Optional[SelfConsistency]): The self-consistency parameters section stored under `ModelMethod`. + logger (BoundLogger): The logger to log messages. Returns: - (Optional[list]): The list of the last two SCF values' magnitudes of a physical property. + (Optional[list]): The list of the last two SCF values (in magnitude) of the physical property. """ + # Initial check + if len(scf_last_steps) != 2: + logger.warning( + '`scf_last_steps` needs to be of length 2, pointing to the last 2 SCF steps performed in the simulation.' + ) + return [] + scf_values = [] for step in scf_last_steps: - scf_phys_property = getattr(step, property_name)[i_property] try: + scf_phys_property = getattr(step, property_name)[i_property] if scf_phys_property.value.u != scf_parameters.threshold_change_unit: logger.error( f'The units of the `scf_step.{property_name}.value` does not coincide with the units of the `self_consistency_ref.threshold_unit`.' @@ -200,36 +211,40 @@ def resolve_is_scf_converged( self, property_name: str, i_property: int, - phys_property: PhysicalProperty, + physical_property: PhysicalProperty, logger: BoundLogger, - ) -> Optional[bool]: + ) -> bool: """ Resolves if the physical property is converged or not after a SCF process. This is only ran when there are at least two `scf_steps` elements. Returns: - (bool): The flag indicating whether the physical property is converged or not after a SCF process. + (bool): Boolean indicating whether the physical property is converged or not after a SCF process. """ - # If there are not at least 2 `scf_steps`, return None + # Check that there are at least 2 `scf_steps` if len(self.scf_steps) < 2: logger.warning('The SCF normalization needs at least two SCF steps.') - return None + return False scf_last_steps = self.scf_steps[-2:] - # If there is not `self_consistency_ref` section, return None - scf_parameters = phys_property.self_consistency_ref + # Check for `self_consistency_ref` section + scf_parameters = physical_property.self_consistency_ref if scf_parameters is None: - return None + return False - # Extract the value.magnitude of the phys_property to be checked if converged or not + # Extract the value.magnitude of the `physical_property` to be checked if converged or not scf_values = self.get_last_scf_steps_value( - scf_last_steps, property_name, i_property, scf_parameters, logger + scf_last_steps=scf_last_steps, + property_name=property_name, + i_property=i_property, + scf_parameters=scf_parameters, + logger=logger, ) if scf_values is None or len(scf_values) != 2: logger.warning( f'The SCF normalization could not resolve the SCF values for the property `{property_name}`.' ) - return None + return False # Compare with the `threshold_change` scf_diff = abs(scf_values[0] - scf_values[1]) @@ -248,10 +263,7 @@ def normalize(self, archive, logger) -> None: # Resolve the `is_scf_converged` flag for all SCF obtained properties for property_name in self.m_def.all_sub_sections.keys(): # Skip the `scf_steps` and `custom_physical_property` sub-sections - if ( - property_name == 'scf_steps' - or property_name == 'custom_physical_property' - ): + if property_name == 'scf_steps': continue # Check if the physical property with that property name is populated @@ -264,5 +276,8 @@ def normalize(self, archive, logger) -> None: # Loop over the physical property of the same m_def type and set `is_scf_converged` for i_property, phys_property in enumerate(phys_properties): phys_property.is_scf_converged = self.resolve_is_scf_converged( - property_name, i_property, phys_property, logger + property_name=property_name, + i_property=i_property, + physical_property=phys_property, + logger=logger, ) diff --git a/tests/conftest.py b/tests/conftest.py index f3929ebe..5dce6b7a 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -159,7 +159,9 @@ def generate_atomic_cell( def generate_scf_electronic_band_gap_template( - threshold_change: float = 1e-3, + n_scf_steps: int = 5, + threshold_change: Optional[float] = 1e-3, + threshold_change_unit: Optional[str] = 'joule', ) -> SCFOutputs: """ Generate a `SCFOutputs` section with a template for the electronic_band_gap property. @@ -167,20 +169,30 @@ def generate_scf_electronic_band_gap_template( scf_outputs = SCFOutputs() # Define a list of scf_steps with values of the total energy like [1, 1.1, 1.11, 1.111, etc], # such that the difference between one step and the next one decreases a factor of 10. - n_scf_steps = 5 - for i in range(1, n_scf_steps): - value = 1 + sum([1 / (10**j) for j in range(1, i + 1)]) - scf_step = Outputs( - electronic_band_gaps=[ElectronicBandGap(value=value * ureg.joule)] - ) + value = None + for i in range(n_scf_steps): + value = 1 + sum([1 / (10**j) for j in range(1, i + 2)]) + scf_step = Outputs(electronic_band_gaps=[ElectronicBandGap(value=value)]) scf_outputs.scf_steps.append(scf_step) # Add a SCF calculated PhysicalProperty - scf_outputs.electronic_band_gaps.append(ElectronicBandGap(value=value * ureg.joule)) + if value is not None: + scf_outputs.electronic_band_gaps.append(ElectronicBandGap(value=value)) + else: + scf_outputs.electronic_band_gaps.append(ElectronicBandGap()) # and a `SelfConsistency` ref section - scf_params = SelfConsistency( - threshold_change=threshold_change, threshold_change_unit='joule' - ) - scf_outputs.electronic_band_gaps[0].self_consistency_ref = scf_params + if threshold_change is not None: + model_method = ModelMethod( + numerical_settings=[ + SelfConsistency( + threshold_change=threshold_change, + threshold_change_unit=threshold_change_unit, + ) + ] + ) + simulation = generate_simulation(model_method=model_method, outputs=scf_outputs) + scf_outputs.electronic_band_gaps[ + 0 + ].self_consistency_ref = simulation.model_method[0].numerical_settings[0] return scf_outputs diff --git a/tests/test_outputs.py b/tests/test_outputs.py index d4efaaa2..2bafc273 100644 --- a/tests/test_outputs.py +++ b/tests/test_outputs.py @@ -17,14 +17,17 @@ # import pytest +from typing import Optional, List -from nomad.units import ureg from nomad.datamodel import EntryArchive from . import logger -from .conftest import generate_scf_electronic_band_gap_template +from .conftest import generate_simulation, generate_scf_electronic_band_gap_template -from nomad_simulations.outputs import Outputs, ElectronicBandGap +from nomad_simulations.model_system import ModelSystem +from nomad_simulations.numerical_settings import SelfConsistency +from nomad_simulations.outputs import Outputs, SCFOutputs +from nomad_simulations.properties import ElectronicBandGap class TestOutputs: @@ -32,61 +35,284 @@ class TestOutputs: Test the `Outputs` class defined in `outputs.py`. """ + def test_number_of_properties(self): + """ + Test how many properties are defined under `Outputs` and its order. This test is done in order to control better + which properties are already defined and in which order to control their normalizations + """ + outputs = Outputs() + assert len(outputs.m_def.all_sub_sections) == 12 + defined_properties = [ + 'fermi_levels', + 'chemical_potentials', + 'crystal_field_splittings', + 'hopping_matrices', + 'electronic_eigenvalues', + 'electronic_band_gaps', + 'electronic_dos', + 'fermi_surfaces', + 'electronic_band_structures', + 'permittivities', + 'absorption_spectra', + 'xas_spectra', + ] + assert list(outputs.m_def.all_sub_sections.keys()) == defined_properties + @pytest.mark.parametrize( - 'threshold_change, result', - [(1e-3, True), (1e-5, False)], + 'band_gaps, values, result_length, result', + [ + # no properties to extract + ([], [], 0, []), + # non-spin polarized case + ([ElectronicBandGap(variables=[])], [2.0], 0, []), + # spin polarized case + ( + [ + ElectronicBandGap(variables=[], spin_channel=0), + ElectronicBandGap(variables=[], spin_channel=1), + ], + [1.0, 1.5], + 2, + [ + ElectronicBandGap(variables=[], spin_channel=0), + ElectronicBandGap(variables=[], spin_channel=1), + ], + ), + ], ) - def test_is_scf_converged(self, threshold_change: float, result: bool): + def test_extract_spin_polarized_properties( + self, + band_gaps: List[ElectronicBandGap], + values: List[float], + result_length: int, + result: List[ElectronicBandGap], + ): """ - Test the `resolve_is_scf_converged` method. + Test the `extract_spin_polarized_property` method. + + Args: + band_gaps (List[ElectronicBandGap]): The `ElectronicBandGap` sections to be stored under `Outputs`. + values (List[float]): The values to be assigned to the `ElectronicBandGap` sections. + result_length (int): The expected length extracted from `extract_spin_polarized_property`. + result (List[ElectronicBandGap]): The expected result of the `extract_spin_polarized_property` method. """ - scf_outputs = generate_scf_electronic_band_gap_template( - threshold_change=threshold_change + outputs = Outputs() + + for i, band_gap in enumerate(band_gaps): + band_gap.value = values[i] + outputs.electronic_band_gaps.append(band_gap) + gaps = outputs.extract_spin_polarized_property( + property_name='electronic_band_gaps' ) - is_scf_converged = scf_outputs.resolve_is_scf_converged( + assert len(gaps) == result_length + if len(result) > 0: + for i, result_gap in enumerate(result): + result_gap.value = values[i] + # ? comparing the sections does not work + assert gaps[i].value == result_gap.value + else: + assert gaps == result + + @pytest.mark.parametrize( + 'model_system', + [(None), (ModelSystem(name='example'))], + ) + def test_set_model_system_ref(self, model_system: Optional[ModelSystem]): + """ + Test the `set_model_system_ref` method. + + Args: + model_system (Optional[ModelSystem]): The `ModelSystem` to be tested for the `model_system_ref` reference + stored in `Outputs`. + """ + outputs = Outputs() + simulation = generate_simulation(model_system=model_system, outputs=outputs) + model_system_ref = outputs.set_model_system_ref() + if model_system is not None: + assert model_system_ref == simulation.model_system[-1] + assert model_system_ref.name == 'example' + else: + assert model_system_ref is None + + @pytest.mark.parametrize( + 'model_system', + [(None), (ModelSystem(name='example'))], + ) + def test_normalize(self, model_system: Optional[ModelSystem]): + """ + Test the `normalize` method. + + Args: + model_system (Optional[ModelSystem]): The expected `model_system_ref` obtained after normalization and + initially stored under `Simulation.model_system[0]`. + """ + outputs = Outputs() + simulation = generate_simulation(model_system=model_system, outputs=outputs) + outputs.normalize(archive=EntryArchive(), logger=logger) + if model_system is not None: + assert outputs.model_system_ref == simulation.model_system[-1] + assert outputs.model_system_ref.name == 'example' + else: + assert outputs.model_system_ref is None + + +class TestSCFOutputs: + """ + Test the `SCFOutputs` class defined in `outputs.py`. + """ + + @pytest.mark.parametrize( + 'scf_last_steps, i_property, values, scf_parameters, result', + [ + # empty `scf_last_steps` + ([], 0, None, None, []), + # length of `scf_last_steps` is different from 2 + ([Outputs()], 0, None, None, []), + # no property matching `'electronic_band_gaps'` stored under the `scf_last_steps` + ([Outputs(), Outputs()], 0, None, None, []), + # `i_property` is out of range + ( + [ + Outputs(electronic_band_gaps=[ElectronicBandGap()]), + Outputs(electronic_band_gaps=[ElectronicBandGap()]), + ], + 2, + None, + None, + [], + ), + # no `value` stored in the `scf_last_steps` property + ( + [ + Outputs(electronic_band_gaps=[ElectronicBandGap()]), + Outputs(electronic_band_gaps=[ElectronicBandGap()]), + ], + 0, + None, + None, + [], + ), + # no `SelfConsistency` section and `threshold_change_unit` defined and macthing units for property `value` (`'joule'`) + ( + [ + Outputs(electronic_band_gaps=[ElectronicBandGap()]), + Outputs(electronic_band_gaps=[ElectronicBandGap()]), + ], + 0, + [1.0, 2.0], + None, + [], + ), + # valid case + ( + [ + Outputs(electronic_band_gaps=[ElectronicBandGap()]), + Outputs(electronic_band_gaps=[ElectronicBandGap()]), + ], + 0, + [1.0, 2.0], + SelfConsistency(threshold_change=1e-6, threshold_change_unit='joule'), + [1.0, 2.0], + ), + ], + ) + def test_get_last_scf_steps_value( + self, + scf_last_steps: List[Outputs], + i_property: int, + values: List[float], + scf_parameters: Optional[SelfConsistency], + result: List[float], + ): + scf_outputs = SCFOutputs() + for i, scf_step in enumerate(scf_last_steps): + property_section = getattr(scf_step, 'electronic_band_gaps') + if property_section is not None and values is not None: + property_section[i_property].value = values[i] + scf_values = scf_outputs.get_last_scf_steps_value( + scf_last_steps=scf_last_steps, property_name='electronic_band_gaps', - i_property=0, - phys_property=scf_outputs.electronic_band_gaps[0], + i_property=i_property, + scf_parameters=scf_parameters, logger=logger, ) - assert is_scf_converged == result + assert scf_values == result - def test_extract_spin_polarized_properties(self): - """ - Test the `extract_spin_polarized_property` method. + @pytest.mark.parametrize( + 'n_scf_steps, threshold_change, property_name, i_property, result', + [ + # `n_scf_steps` is less than 2 + (0, None, '', 0, False), + # no `self_consistency_ref` section + (5, None, '', 0, False), + # no property matching `property_name` + (5, None, 'fermi_levels', 0, False), + # `i_property` is out of range + (5, None, 'electronic_band_gaps', 2, False), + # property is not converged + (5, 1e-5, 'electronic_band_gaps', 0, False), + # valid case: property is converged + (5, 1e-3, 'electronic_band_gaps', 0, True), + ], + ) + def test_resolve_is_scf_converged( + self, + n_scf_steps: int, + threshold_change: Optional[float], + property_name: str, + i_property: int, + result: bool, + ): """ - outputs = Outputs() + Test the `resolve_is_scf_converged` method. - # No spin-polarized band gap - band_gap_non_spin_polarized = ElectronicBandGap(variables=[]) - band_gap_non_spin_polarized.value = 2.0 * ureg.joule - outputs.electronic_band_gaps.append(band_gap_non_spin_polarized) - band_gaps = outputs.extract_spin_polarized_property('electronic_band_gaps') - assert band_gaps == [] - - # Spin-polarized band gaps - band_gap_spin_1 = ElectronicBandGap(variables=[], spin_channel=0) - band_gap_spin_1.value = 1.0 * ureg.joule - outputs.electronic_band_gaps.append(band_gap_spin_1) - band_gap_spin_2 = ElectronicBandGap(variables=[], spin_channel=1) - band_gap_spin_2.value = 1.5 * ureg.joule - outputs.electronic_band_gaps.append(band_gap_spin_2) - band_gaps = outputs.extract_spin_polarized_property('electronic_band_gaps') - assert len(band_gaps) == 2 - assert band_gaps[0].value.magnitude == 1.0 - assert band_gaps[1].value.magnitude == 1.5 + Args: + n_scf_steps (int): The number of SCF steps to add under `SCFOutputs`. + threshold_change (Optional[float]): The threshold change to be used for the SCF convergence. + property_name (str): The name of the property to be tested for convergence. + i_property (int): The index of the property to be tested for convergence. + result (bool): The expected result of the `resolve_is_scf_converged` method. + """ + scf_outputs = generate_scf_electronic_band_gap_template( + n_scf_steps=n_scf_steps, threshold_change=threshold_change + ) + is_scf_converged = scf_outputs.resolve_is_scf_converged( + property_name=property_name, + i_property=i_property, + physical_property=scf_outputs.electronic_band_gaps[0], + logger=logger, + ) + assert is_scf_converged == result @pytest.mark.parametrize( - 'threshold_change, result', - [(1e-3, True), (1e-5, False)], + 'n_scf_steps, threshold_change, result', + [ + # `n_scf_steps` is less than 2 + (0, None, False), + # no `self_consistency_ref` section + (5, None, False), + # property is not converged + (5, 1e-5, False), + # valid case: property is converged + (5, 1e-3, True), + ], ) - def test_normalize(self, threshold_change: float, result: bool): + def test_normalize( + self, + n_scf_steps: int, + threshold_change: float, + result: bool, + ): """ Test the `normalize` method. + + Args: + n_scf_steps (int): The number of SCF steps to add under `SCFOutputs`. + threshold_change (Optional[float]): The threshold change to be used for the SCF convergence. + result (bool): The expected result after normalization and population of the `is_scf_converged` quantity. """ scf_outputs = generate_scf_electronic_band_gap_template( - threshold_change=threshold_change + n_scf_steps=n_scf_steps, threshold_change=threshold_change ) - scf_outputs.normalize(EntryArchive(), logger) assert scf_outputs.electronic_band_gaps[0].is_scf_converged == result From 908a34b1cb3716ffadc3ca759d37120c35e8c905 Mon Sep 17 00:00:00 2001 From: JosePizarro3 Date: Wed, 5 Jun 2024 11:56:44 +0200 Subject: [PATCH 02/12] Moved testing to test_general.py Added explicitly input for methods in ModelSystem and testing Added testing for _set_system_branch_depth and TODO for the functions in these testings which should be implemented in the datamodel --- src/nomad_simulations/general.py | 34 +-- src/nomad_simulations/model_system.py | 62 +++--- tests/conftest.py | 17 +- tests/test_general.py | 292 ++++++++++++++++++++++++++ tests/test_model_system.py | 253 ++++------------------ 5 files changed, 397 insertions(+), 261 deletions(-) create mode 100644 tests/test_general.py diff --git a/src/nomad_simulations/general.py b/src/nomad_simulations/general.py index bbceac87..791addf3 100644 --- a/src/nomad_simulations/general.py +++ b/src/nomad_simulations/general.py @@ -18,18 +18,16 @@ import numpy as np from typing import List -from structlog.stdlib import BoundLogger -from nomad.units import ureg -from nomad.metainfo import SubSection, Quantity, MEnum, Section, Datetime +from nomad.metainfo import SubSection, Quantity, Section, Datetime from nomad.datamodel.metainfo.annotations import ELNAnnotation from nomad.datamodel.data import EntryData from nomad.datamodel.metainfo.basesections import Entity, Activity -from .model_system import ModelSystem -from .model_method import ModelMethod -from .outputs import Outputs -from .utils import is_not_representative, get_composition +from nomad_simulations.model_system import ModelSystem +from nomad_simulations.model_method import ModelMethod +from nomad_simulations.outputs import Outputs +from nomad_simulations.utils import is_not_representative, get_composition class Program(Entity): @@ -178,7 +176,9 @@ def _set_system_branch_depth( ): for system_child in system_parent.model_system: system_child.branch_depth = branch_depth + 1 - self._set_system_branch_depth(system_child, branch_depth + 1) + self._set_system_branch_depth( + system_parent=system_child, branch_depth=branch_depth + 1 + ) def resolve_composition_formula(self, system_parent: ModelSystem) -> None: """Determine and set the composition formula for `system_parent` and all of its @@ -217,7 +217,9 @@ def set_composition_formula( for subsystem in subsystems ] if system.composition_formula is None: - system.composition_formula = get_composition(subsystem_labels) + system.composition_formula = get_composition( + children_names=subsystem_labels + ) def get_composition_recurs(system: ModelSystem, atom_labels: List[str]) -> None: """Traverse the system hierarchy downward and set the branch composition for @@ -229,10 +231,12 @@ def get_composition_recurs(system: ModelSystem, atom_labels: List[str]) -> None: to the atom indices stored in system. """ subsystems = system.model_system - set_composition_formula(system, subsystems, atom_labels) + set_composition_formula( + system=system, subsystems=subsystems, atom_labels=atom_labels + ) if subsystems: for subsystem in subsystems: - get_composition_recurs(subsystem, atom_labels) + get_composition_recurs(system=subsystem, atom_labels=atom_labels) atoms_state = ( system_parent.cell[0].atoms_state if system_parent.cell is not None else [] @@ -242,7 +246,7 @@ def get_composition_recurs(system: ModelSystem, atom_labels: List[str]) -> None: if atoms_state is not None else [] ) - get_composition_recurs(system_parent, atom_labels) + get_composition_recurs(system=system_parent, atom_labels=atom_labels) def normalize(self, archive, logger) -> None: super(EntryData, self).normalize(archive, logger) @@ -263,8 +267,8 @@ def normalize(self, archive, logger) -> None: system_parent.branch_depth = 0 if len(system_parent.model_system) == 0: continue - self._set_system_branch_depth(system_parent) + self._set_system_branch_depth(system_parent=system_parent) - if is_not_representative(system_parent, logger): + if is_not_representative(model_system=system_parent, logger=logger): continue - self.resolve_composition_formula(system_parent) + self.resolve_composition_formula(system_parent=system_parent) diff --git a/src/nomad_simulations/model_system.py b/src/nomad_simulations/model_system.py index 1b6f0842..a1551a29 100644 --- a/src/nomad_simulations/model_system.py +++ b/src/nomad_simulations/model_system.py @@ -19,7 +19,7 @@ import re import numpy as np import ase -from typing import Tuple, Optional, List +from typing import Tuple, Optional from structlog.stdlib import BoundLogger from matid import SymmetryAnalyzer, Classifier # pylint: disable=import-error @@ -39,7 +39,6 @@ Formula, get_normalized_wyckoff, search_aflow_prototype, - get_composition, ) from nomad.metainfo import Quantity, SubSection, SectionProxy, MEnum, Section, Context @@ -47,8 +46,8 @@ from nomad.datamodel.metainfo.basesections import Entity, System from nomad.datamodel.metainfo.annotations import ELNAnnotation -from .atoms_state import AtomsState -from .utils import get_sibling_section, is_not_representative +from nomad_simulations.atoms_state import AtomsState +from nomad_simulations.utils import get_sibling_section, is_not_representative class GeometricSpace(Entity): @@ -185,7 +184,7 @@ def get_geometric_space_for_atomic_cell(self, logger: BoundLogger) -> None: Args: logger (BoundLogger): The logger to log messages. """ - atoms = self.to_ase_atoms(logger) # function defined in AtomicCell + atoms = self.to_ase_atoms(logger=logger) # function defined in AtomicCell cell = atoms.get_cell() self.length_vector_a, self.length_vector_b, self.length_vector_c = ( cell.lengths() * ureg.angstrom @@ -198,7 +197,7 @@ def get_geometric_space_for_atomic_cell(self, logger: BoundLogger) -> None: def normalize(self, archive, logger) -> None: # Skip normalization for `Entity` try: - self.get_geometric_space_for_atomic_cell(logger) + self.get_geometric_space_for_atomic_cell(logger=logger) except Exception: logger.warning( 'Could not extract the geometric space information from ASE Atoms object.', @@ -348,11 +347,11 @@ def to_ase_atoms(self, logger: BoundLogger) -> Optional[ase.Atoms]: 'Could not find `AtomicCell.periodic_boundary_conditions`. They will be set to [False, False, False].' ) self.periodic_boundary_conditions = [False, False, False] - ase_atoms.set_pbc(self.periodic_boundary_conditions) + ase_atoms.set_pbc(pbc=self.periodic_boundary_conditions) # Lattice vectors if self.lattice_vectors is not None: - ase_atoms.set_cell(self.lattice_vectors.to('angstrom').magnitude) + ase_atoms.set_cell(cell=self.lattice_vectors.to('angstrom').magnitude) else: logger.info('Could not find `AtomicCell.lattice_vectors`.') @@ -363,7 +362,9 @@ def to_ase_atoms(self, logger: BoundLogger) -> Optional[ase.Atoms]: 'Length of `AtomicCell.positions` does not coincide with the length of the `AtomicCell.atoms_state`.' ) return None - ase_atoms.set_positions(self.positions.to('angstrom').magnitude) + ase_atoms.set_positions( + newpositions=self.positions.to('angstrom').magnitude + ) else: logger.warning('Could not find `AtomicCell.positions`.') return None @@ -528,7 +529,7 @@ def resolve_analyzed_atomic_cell( ) # ? why do we need to pass units atomic_cell.wyckoff_letters = wyckoff atomic_cell.equivalent_atoms = equivalent_atoms - atomic_cell.get_geometric_space_for_atomic_cell(logger) + atomic_cell.get_geometric_space_for_atomic_cell(logger=logger) return atomic_cell def resolve_bulk_symmetry( @@ -550,7 +551,7 @@ def resolve_bulk_symmetry( """ symmetry = {} try: - ase_atoms = original_atomic_cell.to_ase_atoms(logger) + ase_atoms = original_atomic_cell.to_ase_atoms(logger=logger) symmetry_analyzer = SymmetryAnalyzer( ase_atoms, symmetry_tol=config.normalize.symmetry_tolerance ) @@ -584,12 +585,12 @@ def resolve_bulk_symmetry( # Populating the primitive AtomState information primitive_atomic_cell = self.resolve_analyzed_atomic_cell( - symmetry_analyzer, 'primitive', logger + symmetry_analyzer=symmetry_analyzer, cell_type='primitive', logger=logger ) # Populating the conventional AtomState information conventional_atomic_cell = self.resolve_analyzed_atomic_cell( - symmetry_analyzer, 'conventional', logger + symmetry_analyzer=symmetry_analyzer, cell_type='conventional', logger=logger ) # Getting prototype_formula, prototype_aflow_id, and strukturbericht designation from @@ -606,10 +607,11 @@ def resolve_bulk_symmetry( conventional_wyckoff = conventional_atomic_cell.wyckoff_letters # Normalize wyckoff letters norm_wyckoff = get_normalized_wyckoff( - conventional_num, conventional_wyckoff + atomic_numbers=conventional_num, wyckoff_letters=conventional_wyckoff ) aflow_prototype = search_aflow_prototype( - symmetry.get('space_group_number'), norm_wyckoff + space_group=symmetry.get('space_group_number'), + norm_wyckoff=norm_wyckoff, ) if aflow_prototype: strukturbericht = aflow_prototype.get('Strukturbericht Designation') @@ -642,7 +644,9 @@ def normalize(self, archive, logger) -> None: ( primitive_atomic_cell, conventional_atomic_cell, - ) = self.resolve_bulk_symmetry(atomic_cell, logger) + ) = self.resolve_bulk_symmetry( + original_atomic_cell=atomic_cell, logger=logger + ) self.m_parent.m_add_sub_section(ModelSystem.cell, primitive_atomic_cell) self.m_parent.m_add_sub_section(ModelSystem.cell, conventional_atomic_cell) # Reference to the standarized cell, and if not, fallback to the originally parsed one @@ -712,11 +716,11 @@ def resolve_chemical_formulas(self, formula: Formula) -> None: Args: formula (Formula): The Formula object from NOMAD atomutils containing the chemical formulas. """ - self.descriptive = formula.format('descriptive') - self.reduced = formula.format('reduced') - self.iupac = formula.format('iupac') - self.hill = formula.format('hill') - self.anonymous = formula.format('anonymous') + self.descriptive = formula.format(fmt='descriptive') + self.reduced = formula.format(fmt='reduced') + self.iupac = formula.format(fmt='iupac') + self.hill = formula.format(fmt='hill') + self.anonymous = formula.format(fmt='anonymous') def normalize(self, archive, logger) -> None: super().normalize(archive, logger) @@ -727,10 +731,10 @@ def normalize(self, archive, logger) -> None: if atomic_cell is None: logger.warning('Could not resolve the sibling `AtomicCell` section.') return - ase_atoms = atomic_cell.to_ase_atoms(logger) + ase_atoms = atomic_cell.to_ase_atoms(logger=logger) formula = None try: - formula = Formula(ase_atoms.get_chemical_formula()) + formula = Formula(formula=ase_atoms.get_chemical_formula()) # self.chemical_composition = ase_atoms.get_chemical_formula(mode="all") except ValueError as e: logger.warning( @@ -739,7 +743,7 @@ def normalize(self, archive, logger) -> None: error=str(e), ) if formula: - self.resolve_chemical_formulas(formula) + self.resolve_chemical_formulas(formula=formula) self.m_cache['elemental_composition'] = formula.elemental_composition() @@ -957,7 +961,7 @@ def resolve_system_type_and_dimensionality( radii='covalent', cluster_threshold=config.normalize.cluster_threshold, ) - classification = classifier.classify(ase_atoms) + classification = classifier.classify(input_system=ase_atoms) except Exception as e: logger.warning( 'MatID system classification failed.', exc_info=e, error=str(e) @@ -993,7 +997,7 @@ def normalize(self, archive, logger) -> None: super().normalize(archive, logger) # We don't need to normalize if the system is not representative - if is_not_representative(self, logger): + if is_not_representative(model_system=self, logger=logger): return # Extracting ASE Atoms object from the originally parsed AtomicCell section @@ -1004,7 +1008,7 @@ def normalize(self, archive, logger) -> None: return if self.cell[0].name == 'AtomicCell': self.cell[0].type = 'original' - ase_atoms = self.cell[0].to_ase_atoms(logger) + ase_atoms = self.cell[0].to_ase_atoms(logger=logger) if not ase_atoms: return @@ -1016,7 +1020,9 @@ def normalize(self, archive, logger) -> None: ( self.type, self.dimensionality, - ) = self.resolve_system_type_and_dimensionality(ase_atoms, logger) + ) = self.resolve_system_type_and_dimensionality( + ase_atoms=ase_atoms, logger=logger + ) # Creating and normalizing Symmetry section if self.type == 'bulk' and self.symmetry is not None: sec_symmetry = self.m_create(Symmetry) diff --git a/tests/conftest.py b/tests/conftest.py index 5dce6b7a..2f0baf04 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -120,11 +120,11 @@ def generate_model_system( def generate_atomic_cell( - lattice_vectors: List = [[1, 0, 0], [0, 1, 0], [0, 0, 1]], - positions=None, - periodic_boundary_conditions=None, - chemical_symbols: List = ['H', 'H', 'O'], - atomic_numbers: List = [1, 1, 8], + lattice_vectors: List[List[float]] = [[1, 0, 0], [0, 1, 0], [0, 0, 1]], + positions: Optional[list] = None, + periodic_boundary_conditions: List[bool] = [False, False, False], + chemical_symbols: List[str] = ['H', 'H', 'O'], + atomic_numbers: List[int] = [1, 1, 8], ) -> AtomicCell: """ Generate an `AtomicCell` section with the given parameters. @@ -133,18 +133,13 @@ def generate_atomic_cell( if positions is None and chemical_symbols is not None: n_atoms = len(chemical_symbols) positions = [[i / n_atoms, i / n_atoms, i / n_atoms] for i in range(n_atoms)] - # Define periodic boundary conditions if not provided - if periodic_boundary_conditions is None: - periodic_boundary_conditions = [False, False, False] # Define the atomic cell - atomic_cell = AtomicCell() + atomic_cell = AtomicCell(periodic_boundary_conditions=periodic_boundary_conditions) if lattice_vectors: atomic_cell.lattice_vectors = lattice_vectors * ureg('angstrom') if positions: atomic_cell.positions = positions * ureg('angstrom') - if periodic_boundary_conditions: - atomic_cell.periodic_boundary_conditions = periodic_boundary_conditions # Add the elements information for index, atom in enumerate(chemical_symbols): diff --git a/tests/test_general.py b/tests/test_general.py new file mode 100644 index 00000000..8d83b3c6 --- /dev/null +++ b/tests/test_general.py @@ -0,0 +1,292 @@ +# +# Copyright The NOMAD Authors. +# +# This file is part of NOMAD. See https://nomad-lab.eu for further info. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +# + +import pytest +import numpy as np +from typing import List + +from nomad.datamodel import EntryArchive + +from . import logger + +from nomad_simulations.general import Simulation +from nomad_simulations.model_system import ( + ModelSystem, + AtomicCell, + AtomsState, +) + + +class TestSimulation: + """ + Test the `Simulation` class defined in general.py + """ + + @pytest.mark.parametrize( + 'system, result', + [ + ([ModelSystem(name='depth 0')], [0]), + ( + [ + ModelSystem( + name='depth 0', + model_system=[ + ModelSystem(name='depth 1'), + ModelSystem(name='depth 1'), + ], + ) + ], + [0, 1, 1], + ), + ( + [ + ModelSystem( + name='depth 0', + model_system=[ + ModelSystem( + name='depth 1', + model_system=[ModelSystem(name='depth 2')], + ), + ModelSystem(name='depth 1'), + ], + ) + ], + [0, 1, 2, 1], + ), + ], + ) + def test_set_system_branch_depth( + self, system: List[ModelSystem], result: List[int] + ): + """ + Test the `_set_system_branch_depth` method. + + Args: + system (List[ModelSystem]): The system hierarchy to set the branch depths for. + result (List[int]): The expected branch depths for each system in the hierarchy. + """ + simulation = Simulation(model_system=system) + for system_parent in simulation.model_system: + system_parent.branch_depth = 0 + if len(system_parent.model_system) == 0: + continue + simulation._set_system_branch_depth(system_parent=system_parent) + + # TODO move this into its own method to handle `ModelSystem` hierarchies (see below `get_system_recurs`) + def get_flat_depths( + system_parent: ModelSystem, quantity_name: str, value: list = [] + ): + for system_child in system_parent.model_system: + val = getattr(system_child, quantity_name) + value.append(val) + get_flat_depths( + system_parent=system_child, quantity_name=quantity_name, value=value + ) + return value + + value = get_flat_depths( + system_parent=simulation.model_system[0], + quantity_name='branch_depth', + value=[0], + ) + assert value == result + + @pytest.mark.parametrize( + 'is_representative, has_atom_indices, mol_label_list, n_mol_list, atom_labels_list, composition_formula_list, custom_formulas', + [ + ( + True, + True, + ['H20'], + [3], + [['H', 'O', 'O']], + ['group_H20(1)', 'H20(3)', 'H(1)O(2)', 'H(1)O(2)', 'H(1)O(2)'], + [None, None, None, None, None], + ), # pure system + ( + False, + True, + ['H20'], + [3], + [['H', 'O', 'O']], + [None, None, None, None, None], + [None, None, None, None, None], + ), # non-representative system + ( + True, + True, + [None], + [3], + [['H', 'O', 'O']], + ['Unknown(1)', 'Unknown(3)', 'H(1)O(2)', 'H(1)O(2)', 'H(1)O(2)'], + [None, None, None, None, None], + ), # missing branch labels + ( + True, + True, + ['H20'], + [3], + [[None, None, None]], + ['group_H20(1)', 'H20(3)', 'Unknown(3)', 'Unknown(3)', 'Unknown(3)'], + [None, None, None, None, None], + ), # missing atom labels + ( + True, + False, + ['H20'], + [3], + [['H', 'O', 'O']], + ['group_H20(1)', 'H20(3)', None, None, None], + [None, None, None, None, None], + ), # missing atom indices + ( + True, + True, + ['H20'], + [3], + [['H', 'O', 'O']], + ['waters(1)', 'water_molecules(3)', 'H(1)O(2)', 'H(1)O(2)', 'H(1)O(2)'], + ['waters(1)', 'water_molecules(3)', None, None, None], + ), # custom formulas + ( + True, + True, + ['H20', 'Methane'], + [5, 2], + [['H', 'O', 'O'], ['C', 'H', 'H', 'H', 'H']], + [ + 'group_H20(1)group_Methane(1)', + 'H20(5)', + 'H(1)O(2)', + 'H(1)O(2)', + 'H(1)O(2)', + 'H(1)O(2)', + 'H(1)O(2)', + 'Methane(2)', + 'C(1)H(4)', + 'C(1)H(4)', + ], + [None, None, None, None, None, None, None, None, None, None], + ), # binary mixture + ], + ) + def test_system_hierarchy_for_molecules( + self, + is_representative: bool, + has_atom_indices: bool, + mol_label_list: List[str], + n_mol_list: List[int], + atom_labels_list: List[str], + composition_formula_list: List[str], + custom_formulas: List[str], + ): + """ + Test the `Simulation` normalization for obtaining `Model.System.composition_formula` for atoms and molecules. + + Args: + is_representative (bool): Specifies if branch_depth = 0 is representative or not. + If not representative, the composition formulas should not be generated. + has_atom_indices (bool): Specifies if the atom_indices should be populated during parsing. + Without atom_indices, the composition formulas for the deepest level of the hierarchy + should not be populated. + mol_label_list (List[str]): Molecule types for generating the hierarchy. + n_mol_list (List[int]): Number of molecules for each molecule type. Should be same + length as mol_label_list. + atom_labels_list (List[str]): Atom labels for each molecule type. Should be same length as + mol_label_list, with each entry being a list of corresponding atom labels. + composition_formula_list (List[str]): Resulting composition formulas after normalization. The + ordering is dictated by the recursive traversing of the hierarchy in get_system_recurs(), + which follows each branch to its deepest level before moving to the next branch, i.e., + [model_system.composition_formula, + model_system.model_system[0].composition_formula], + model_system.model_system[0].model_system[0].composition_formula, + model_system.model_system[0].model_system[1].composition_formula, ..., + model_system.model_system[1].composition_formula, ...] + custom_formulas (List[str]): Custom composition formulas that can be set in the generation + of the hierarchy, which will cause the normalize to ignore (i.e., not overwrite) these formula entries. + The ordering is as described above. + """ + + ### Generate the system hierarchy ### + simulation = Simulation() + model_system = ModelSystem(is_representative=True) + simulation.model_system.append(model_system) + model_system.branch_label = 'Total System' + model_system.is_representative = is_representative + model_system.composition_formula = custom_formulas[0] + ctr_comp = 1 + atomic_cell = AtomicCell() + model_system.cell.append(atomic_cell) + if has_atom_indices: + model_system.atom_indices = [] + for mol_label, n_mol, atom_labels in zip( + mol_label_list, n_mol_list, atom_labels_list + ): + # Create a branch in the hierarchy for this molecule type + model_system_mol_group = ModelSystem() + if has_atom_indices: + model_system_mol_group.atom_indices = [] + model_system_mol_group.branch_label = ( + f'group_{mol_label}' if mol_label is not None else None + ) + model_system_mol_group.composition_formula = custom_formulas[ctr_comp] + ctr_comp += 1 + model_system.model_system.append(model_system_mol_group) + for _ in range(n_mol): + # Create a branch in the hierarchy for this molecule + model_system_mol = ModelSystem(branch_label=mol_label) + model_system_mol.branch_label = mol_label + model_system_mol.composition_formula = custom_formulas[ctr_comp] + ctr_comp += 1 + model_system_mol_group.model_system.append(model_system_mol) + # add the corresponding atoms to the global atom list + for atom_label in atom_labels: + if atom_label is not None: + atomic_cell.atoms_state.append( + AtomsState(chemical_symbol=atom_label) + ) + n_atoms = len(atomic_cell.atoms_state) + atom_indices = np.arange(n_atoms - len(atom_labels), n_atoms) + if has_atom_indices: + model_system_mol.atom_indices = atom_indices + model_system_mol_group.atom_indices = np.append( + model_system_mol_group.atom_indices, atom_indices + ) + model_system.atom_indices = np.append( + model_system.atom_indices, atom_indices + ) + + simulation.normalize(EntryArchive(), logger) + + ### Traverse the hierarchy recursively and check the results ### + assert model_system.composition_formula == composition_formula_list[0] + ctr_comp = 1 + + # TODO move this into its own method to handle `ModelSystem` hierarchies (see above `get_flat_depths`) + def get_system_recurs(systems: List[ModelSystem], ctr_comp: int) -> int: + for sys in systems: + assert sys.composition_formula == composition_formula_list[ctr_comp] + ctr_comp += 1 + subsystems = sys.model_system + if subsystems: + ctr_comp = get_system_recurs(subsystems, ctr_comp) + return ctr_comp + + new_ctr_comp = get_system_recurs( + systems=model_system.model_system, ctr_comp=ctr_comp + ) diff --git a/tests/test_model_system.py b/tests/test_model_system.py index 34caf9c3..c317daf7 100644 --- a/tests/test_model_system.py +++ b/tests/test_model_system.py @@ -25,14 +25,7 @@ from . import logger from .conftest import generate_atomic_cell -from nomad_simulations.model_system import ( - Symmetry, - ChemicalFormula, - ModelSystem, - AtomicCell, - AtomsState, -) -from nomad_simulations.general import Simulation +from nomad_simulations.model_system import Symmetry, ChemicalFormula, ModelSystem class TestAtomicCell: @@ -96,17 +89,25 @@ def test_generate_ase_atoms( ): """ Test the creation of `ase.Atoms` from `AtomicCell`. + + Args: + chemical_symbols (List[str]): List of chemical symbols. + atomic_numbers (List[int]): List of atomic numbers. + formula (str): Chemical formula. + lattice_vectors (List[List[float]]): Lattice vectors. + positions (List[List[float]]): Atomic positions. + periodic_boundary_conditions (List[bool]): Periodic boundary conditions. """ atomic_cell = generate_atomic_cell( - lattice_vectors, - positions, - periodic_boundary_conditions, - chemical_symbols, - atomic_numbers, + lattice_vectors=lattice_vectors, + positions=positions, + periodic_boundary_conditions=periodic_boundary_conditions, + chemical_symbols=chemical_symbols, + atomic_numbers=atomic_numbers, ) # Test `to_ase_atoms` function - ase_atoms = atomic_cell.to_ase_atoms(logger) + ase_atoms = atomic_cell.to_ase_atoms(logger=logger) if not chemical_symbols or len(chemical_symbols) != len(positions): assert ase_atoms is None else: @@ -190,14 +191,21 @@ def test_geometric_space( ): """ Test the `GeometricSpace` quantities normalization from `AtomicCell`. + + Args: + chemical_symbols (List[str]): List of chemical symbols. + atomic_numbers (List[int]): List of atomic numbers. + lattice_vectors (List[List[float]]): Lattice vectors. + positions (List[List[float]]): Atomic positions. + vectors_results (List[Optional[float]]): Expected lengths of cell vectors. + angles_results (List[Optional[float]]): Expected angles between cell vectors. + volume (Optional[float]): Expected volume of the cell. """ - pbc = [False, False, False] atomic_cell = generate_atomic_cell( - lattice_vectors, - positions, - pbc, - chemical_symbols, - atomic_numbers, + lattice_vectors=lattice_vectors, + positions=positions, + chemical_symbols=chemical_symbols, + atomic_numbers=atomic_numbers, ) # Get `GeometricSpace` quantities via normalization of `AtomicCell` @@ -275,6 +283,11 @@ def test_chemical_formula( ): """ Test the `ChemicalFormula` normalization if a sibling `AtomicCell` is created, and thus the `Formula` class can be used. + + Args: + chemical_symbols (List[str]): List of chemical symbols. + atomic_numbers (List[int]): List of atomic numbers. + formulas (List[str]): List of expected formulas. """ atomic_cell = generate_atomic_cell( chemical_symbols=chemical_symbols, atomic_numbers=atomic_numbers @@ -332,17 +345,25 @@ def test_system_type_and_dimensionality( ): """ Test the `ModelSystem` normalization of `type` and `dimensionality` from `AtomicCell`. + + Args: + positions (List[List[float]]): Atomic positions. + pbc (Optional[List[bool]]): Periodic boundary conditions. + system_type (str): Expected system type. + dimensionality (int): Expected dimensionality. """ atomic_cell = generate_atomic_cell( positions=positions, periodic_boundary_conditions=pbc ) - ase_atoms = atomic_cell.to_ase_atoms(logger) + ase_atoms = atomic_cell.to_ase_atoms(logger=logger) model_system = ModelSystem() model_system.cell.append(atomic_cell) ( resolved_system_type, resolved_dimensionality, - ) = model_system.resolve_system_type_and_dimensionality(ase_atoms, logger) + ) = model_system.resolve_system_type_and_dimensionality( + ase_atoms=ase_atoms, logger=logger + ) assert resolved_system_type == system_type assert resolved_dimensionality == dimensionality @@ -360,7 +381,9 @@ def test_symmetry(self): ) ).all() symmetry = Symmetry() - primitive, conventional = symmetry.resolve_bulk_symmetry(atomic_cell, logger) + primitive, conventional = symmetry.resolve_bulk_symmetry( + original_atomic_cell=atomic_cell, logger=logger + ) assert symmetry.bravais_lattice == 'hR' assert symmetry.hall_symbol == '-R 3 2"' assert symmetry.point_group_symbol == '-3m' @@ -442,187 +465,3 @@ def test_normalize(self): assert np.isclose(model_system.elemental_composition[0].atomic_fraction, 2 / 3) assert model_system.elemental_composition[1].element == 'O' assert np.isclose(model_system.elemental_composition[1].atomic_fraction, 1 / 3) - - @pytest.mark.parametrize( - 'is_representative, has_atom_indices, mol_label_list, n_mol_list, atom_labels_list, composition_formula_list, custom_formulas', - [ - ( - True, - True, - ['H20'], - [3], - [['H', 'O', 'O']], - ['group_H20(1)', 'H20(3)', 'H(1)O(2)', 'H(1)O(2)', 'H(1)O(2)'], - [None, None, None, None, None], - ), # pure system - ( - False, - True, - ['H20'], - [3], - [['H', 'O', 'O']], - [None, None, None, None, None], - [None, None, None, None, None], - ), # non-representative system - ( - True, - True, - [None], - [3], - [['H', 'O', 'O']], - ['Unknown(1)', 'Unknown(3)', 'H(1)O(2)', 'H(1)O(2)', 'H(1)O(2)'], - [None, None, None, None, None], - ), # missing branch labels - ( - True, - True, - ['H20'], - [3], - [[None, None, None]], - ['group_H20(1)', 'H20(3)', 'Unknown(3)', 'Unknown(3)', 'Unknown(3)'], - [None, None, None, None, None], - ), # missing atom labels - ( - True, - False, - ['H20'], - [3], - [['H', 'O', 'O']], - ['group_H20(1)', 'H20(3)', None, None, None], - [None, None, None, None, None], - ), # missing atom indices - ( - True, - True, - ['H20'], - [3], - [['H', 'O', 'O']], - ['waters(1)', 'water_molecules(3)', 'H(1)O(2)', 'H(1)O(2)', 'H(1)O(2)'], - ['waters(1)', 'water_molecules(3)', None, None, None], - ), # custom formulas - ( - True, - True, - ['H20', 'Methane'], - [5, 2], - [['H', 'O', 'O'], ['C', 'H', 'H', 'H', 'H']], - [ - 'group_H20(1)group_Methane(1)', - 'H20(5)', - 'H(1)O(2)', - 'H(1)O(2)', - 'H(1)O(2)', - 'H(1)O(2)', - 'H(1)O(2)', - 'Methane(2)', - 'C(1)H(4)', - 'C(1)H(4)', - ], - [None, None, None, None, None, None, None, None, None, None], - ), # binary mixture - ], - ) - def test_system_hierarchy_for_molecules( - self, - is_representative: bool, - has_atom_indices: bool, - mol_label_list: List[str], - n_mol_list: List[int], - atom_labels_list: List[str], - composition_formula_list: List[str], - custom_formulas: List[str], - ): - """Test the `ModelSystem` normalization of 'composition_formula' for atoms and molecules. - - Args: - is_representative (bool): Specifies if branch_depth = 0 is representative or not. - If not representative, the composition formulas should not be generated. - has_atom_indices (bool): Specifies if the atom_indices should be populated during parsing. - Without atom_indices, the composition formulas for the deepest level of the hierarchy - should not be populated. - mol_label_list (List[str]): Molecule types for generating the hierarchy. - n_mol_list (List[int]): Number of molecules for each molecule type. Should be same - length as mol_label_list. - atom_labels_list (List[str]): Atom labels for each molecule type. Should be same length as - mol_label_list, with each entry being a list of corresponding atom labels. - composition_formula_list (List[str]): Resulting composition formulas after normalization. The - ordering is dictated by the recursive traversing of the hierarchy in get_system_recurs(), - which follows each branch to its deepest level before moving to the next branch, i.e., - [model_system.composition_formula, - model_system.model_system[0].composition_formula], - model_system.model_system[0].model_system[0].composition_formula, - model_system.model_system[0].model_system[1].composition_formula, ..., - model_system.model_system[1].composition_formula, ...] - custom_formulas (List[str]): Custom composition formulas that can be set in the generation - of the hierarchy, which will cause the normalize to ignore (i.e., not overwrite) these formula entries. - The ordering is as described above. - - Returns: - None - """ - - ### Generate the system hierarchy ### - simulation = Simulation() - model_system = ModelSystem(is_representative=True) - simulation.model_system.append(model_system) - model_system.branch_label = 'Total System' - model_system.is_representative = is_representative - model_system.composition_formula = custom_formulas[0] - ctr_comp = 1 - atomic_cell = AtomicCell() - model_system.cell.append(atomic_cell) - if has_atom_indices: - model_system.atom_indices = [] - for mol_label, n_mol, atom_labels in zip( - mol_label_list, n_mol_list, atom_labels_list - ): - # Create a branch in the hierarchy for this molecule type - model_system_mol_group = ModelSystem() - if has_atom_indices: - model_system_mol_group.atom_indices = [] - model_system_mol_group.branch_label = ( - f'group_{mol_label}' if mol_label is not None else None - ) - model_system_mol_group.composition_formula = custom_formulas[ctr_comp] - ctr_comp += 1 - model_system.model_system.append(model_system_mol_group) - for _ in range(n_mol): - # Create a branch in the hierarchy for this molecule - model_system_mol = ModelSystem(branch_label=mol_label) - model_system_mol.branch_label = mol_label - model_system_mol.composition_formula = custom_formulas[ctr_comp] - ctr_comp += 1 - model_system_mol_group.model_system.append(model_system_mol) - # add the corresponding atoms to the global atom list - for atom_label in atom_labels: - if atom_label is not None: - atomic_cell.atoms_state.append( - AtomsState(chemical_symbol=atom_label) - ) - n_atoms = len(atomic_cell.atoms_state) - atom_indices = np.arange(n_atoms - len(atom_labels), n_atoms) - if has_atom_indices: - model_system_mol.atom_indices = atom_indices - model_system_mol_group.atom_indices = np.append( - model_system_mol_group.atom_indices, atom_indices - ) - model_system.atom_indices = np.append( - model_system.atom_indices, atom_indices - ) - - simulation.normalize(EntryArchive(), logger) - - ### Traverse the hierarchy recursively and check the results ### - assert model_system.composition_formula == composition_formula_list[0] - ctr_comp = 1 - - def get_system_recurs(sec_system, ctr_comp): - for sys in sec_system: - assert sys.composition_formula == composition_formula_list[ctr_comp] - ctr_comp += 1 - sec_subsystem = sys.model_system - if sec_subsystem: - ctr_comp = get_system_recurs(sec_subsystem, ctr_comp) - return ctr_comp - - get_system_recurs(model_system.model_system, ctr_comp) From f3b2b437919d12102e0e0f1aa2c75f79be3c3440 Mon Sep 17 00:00:00 2001 From: JosePizarro3 Date: Wed, 5 Jun 2024 12:22:32 +0200 Subject: [PATCH 03/12] Added explicit input and fix testing for atoms_state.py --- src/nomad_simulations/atoms_state.py | 18 ++++----- tests/conftest.py | 2 +- tests/test_atoms_state.py | 59 ++++++++++++++++++++++++---- 3 files changed, 61 insertions(+), 18 deletions(-) diff --git a/src/nomad_simulations/atoms_state.py b/src/nomad_simulations/atoms_state.py index 392f4edd..6b32f43e 100644 --- a/src/nomad_simulations/atoms_state.py +++ b/src/nomad_simulations/atoms_state.py @@ -28,7 +28,7 @@ from nomad.datamodel.metainfo.basesections import Entity from nomad.datamodel.metainfo.annotations import ELNAnnotation -from .utils import RussellSaundersState +from nomad_simulations.utils import RussellSaundersState class OrbitalsState(Entity): @@ -280,7 +280,7 @@ def resolve_degeneracy(self) -> Optional[int]: for jj in self.j_quantum_number: if self.mj_quantum_number is not None: mjs = RussellSaundersState.generate_MJs( - self.j_quantum_number[0], rising=True + J=self.j_quantum_number[0], rising=True ) degeneracy += len( [mj for mj in mjs if mj in self.mj_quantum_number] @@ -293,7 +293,7 @@ def normalize(self, archive, logger) -> None: super().normalize(archive, logger) # General checks for physical quantum numbers and symbols - if not self.validate_quantum_numbers(logger): + if not self.validate_quantum_numbers(logger=logger): logger.error('The quantum numbers are not physical.') return @@ -301,7 +301,7 @@ def normalize(self, archive, logger) -> None: for quantum_name in ['l', 'ml', 'ms']: for quantum_type in ['number', 'symbol']: quantity = self.resolve_number_and_symbol( - quantum_name, quantum_type, logger + quantum_name=quantum_name, quantum_type=quantum_type, logger=logger ) if getattr(self, f'{quantum_name}_quantum_{quantum_type}') is None: setattr(self, f'{quantum_name}_quantum_{quantum_type}', quantity) @@ -383,7 +383,7 @@ def normalize(self, archive, logger) -> None: self.n_excited_electrons = None self.orbital_ref.degeneracy = 1 if self.orbital_ref.occupation is None: - self.orbital_ref.occupation = self.resolve_occupation(logger) + self.orbital_ref.occupation = self.resolve_occupation(logger=logger) class HubbardInteractions(ArchiveSection): @@ -552,11 +552,11 @@ def normalize(self, archive, logger) -> None: self.u_interaction, self.u_interorbital_interaction, self.j_hunds_coupling, - ) = self.resolve_u_interactions(logger) + ) = self.resolve_u_interactions(logger=logger) # If u_effective is not available, calculate it if self.u_effective is None: - self.u_effective = self.resolve_u_effective(logger) + self.u_effective = self.resolve_u_effective(logger=logger) # Check if length of `orbitals_ref` is the same as the length of `umn`: if self.u_matrix is not None and self.orbitals_ref is not None: @@ -652,6 +652,6 @@ def normalize(self, archive, logger) -> None: # Get chemical_symbol from atomic_number and viceversa if self.chemical_symbol is None: - self.chemical_symbol = self.resolve_chemical_symbol(logger) + self.chemical_symbol = self.resolve_chemical_symbol(logger=logger) if self.atomic_number is None: - self.atomic_number = self.resolve_atomic_number(logger) + self.atomic_number = self.resolve_atomic_number(logger=logger) diff --git a/tests/conftest.py b/tests/conftest.py index 2f0baf04..96ba526f 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -145,7 +145,7 @@ def generate_atomic_cell( for index, atom in enumerate(chemical_symbols): atom_state = AtomsState() setattr(atom_state, 'chemical_symbol', atom) - atomic_number = atom_state.resolve_atomic_number(logger) + atomic_number = atom_state.resolve_atomic_number(logger=logger) assert atomic_number == atomic_numbers[index] atom_state.atomic_number = atomic_number atomic_cell.atoms_state.append(atom_state) diff --git a/tests/test_atoms_state.py b/tests/test_atoms_state.py index 4482d1ea..d088c144 100644 --- a/tests/test_atoms_state.py +++ b/tests/test_atoms_state.py @@ -69,13 +69,18 @@ def test_validate_quantum_numbers( ): """ Test the `validate_quantum_numbers` method. + + Args: + number_label (str): The quantum number string to be tested. + values (List[int]): The values stored in `OrbitalState`. + results (List[bool]): The expected results after validation. """ orbital_state = OrbitalsState(n_quantum_number=2) for val, res in zip(values, results): if number_label == 'ml_quantum_number': orbital_state.l_quantum_number = 2 setattr(orbital_state, number_label, val) - assert orbital_state.validate_quantum_numbers(logger) == res + assert orbital_state.validate_quantum_numbers(logger=logger) == res @pytest.mark.parametrize( 'quantum_name, value, expected_result', @@ -103,6 +108,11 @@ def test_number_and_symbol( ): """ Test the number and symbol resolution for each of the quantum numbers defined in the parametrization. + + Args: + quantum_name (str): The quantum number string to be tested. + value (Union[int, float]): The value stored in `OrbitalState`. + expected_result (Optional[str]): The expected result after resolving the counter-type. """ # Adding quantum numbers to the `OrbitalsState` section orbital_state = OrbitalsState(n_quantum_number=2) @@ -112,13 +122,13 @@ def test_number_and_symbol( # Making sure that the `'number'` is assigned resolved_type = orbital_state.resolve_number_and_symbol( - quantum_name, 'number', logger + quantum_name=quantum_name, quantum_type='number', logger=logger ) assert resolved_type == value # Resolving if the counter-type is assigned resolved_countertype = orbital_state.resolve_number_and_symbol( - quantum_name, 'symbol', logger + quantum_name=quantum_name, quantum_type='symbol', logger=logger ) assert resolved_countertype == expected_result @@ -146,6 +156,14 @@ def test_degeneracy( ): """ Test the degeneracy of each orbital states defined in the parametrization. + + Args: + l_quantum_number (int): The angular momentum quantum number. + ml_quantum_number (Optional[int]): The magnetic quantum number. + j_quantum_number (Optional[List[float]]): The total angular momentum quantum number. + mj_quantum_number (Optional[List[float]]): The magnetic quantum number for the total angular momentum. + ms_quantum_number (Optional[float]): The spin quantum number. + degeneracy (int): The expected degeneracy of the orbital state. """ orbital_state = OrbitalsState(n_quantum_number=2) self.add_state( @@ -195,13 +213,19 @@ def test_occupation( ): """ Test the occupation of a core hole for a given set of orbital reference and degeneracy. + + Args: + orbital_ref (Optional[OrbitalsState]): The orbital reference of the core hole. + degeneracy (Optional[int]): The degeneracy of the orbital reference. + n_excited_electrons (float): The number of excited electrons. + occupation (Optional[float]): The expected occupation of the core hole. """ core_hole = CoreHole( orbital_ref=orbital_ref, n_excited_electrons=n_excited_electrons ) if orbital_ref is not None: assert orbital_ref.resolve_degeneracy() == degeneracy - resolved_occupation = core_hole.resolve_occupation(logger) + resolved_occupation = core_hole.resolve_occupation(logger=logger) if resolved_occupation is not None: assert np.isclose(resolved_occupation, occupation) else: @@ -232,6 +256,12 @@ def test_normalize( ): """ Test the normalization of the `CoreHole`. Inputs are defined as the quantities of the `CoreHole` section. + + Args: + orbital_ref (Optional[OrbitalsState]): The orbital reference of the core hole. + n_excited_electrons (Optional[float]): The number of excited electrons. + dscf_state (Optional[str]): The DSCF state of the core hole. + results (Tuple[Optional[float], Optional[float], Optional[float]]): The expected results after normalization. """ core_hole = CoreHole( orbital_ref=orbital_ref, @@ -265,6 +295,10 @@ def test_u_interactions( ): """ Test the Hubbard interactions `U`, `U'`, and `J` for a given set of Slater integrals. + + Args: + slater_integrals (Optional[List[float]]): The Slater integrals of the Hubbard interactions. + results (Tuple[Optional[float], Optional[float], Optional[float]]): The expected results of the Hubbard interactions. """ # Adding `slater_integrals` to the `HubbardInteractions` section hubbard_interactions = HubbardInteractions() @@ -276,7 +310,7 @@ def test_u_interactions( u_interaction, u_interorbital_interaction, j_hunds_coupling, - ) = hubbard_interactions.resolve_u_interactions(logger) + ) = hubbard_interactions.resolve_u_interactions(logger=logger) if None not in (u_interaction, u_interorbital_interaction, j_hunds_coupling): assert np.isclose(u_interaction.to('eV').magnitude, results[0]) @@ -306,6 +340,11 @@ def test_u_effective( ): """ Test the effective Hubbard interaction `U_eff` for a given set of Hubbard interactions `U` and `J`. + + Args: + u_interaction (Optional[float]): The Hubbard interaction `U`. + j_local_exchange_interaction (Optional[float]): The Hubbard interaction `J`. + u_effective (Optional[float]): The expected effective Hubbard interaction `U_eff`. """ # Adding `u_interaction` and `j_local_exchange_interaction` to the `HubbardInteractions` section hubbard_interactions = HubbardInteractions() @@ -317,7 +356,7 @@ def test_u_effective( ) # Resolving Ueff from class method - resolved_u_effective = hubbard_interactions.resolve_u_effective(logger) + resolved_u_effective = hubbard_interactions.resolve_u_effective(logger=logger) if resolved_u_effective is not None: assert np.isclose(resolved_u_effective.to('eV').magnitude, u_effective) else: @@ -358,10 +397,14 @@ def test_chemical_symbol_and_atomic_number( ): """ Test the `chemical_symbol` and `atomic_number` resolution for the `AtomsState` section. + + Args: + chemical_symbol (str): The chemical symbol of the atom. + atomic_number (int): The atomic number of the atom. """ # Testing `chemical_symbol` atom_state = AtomsState(chemical_symbol=chemical_symbol) - assert atom_state.resolve_atomic_number(logger) == atomic_number + assert atom_state.resolve_atomic_number(logger=logger) == atomic_number # Testing `atomic_number` atom_state.atomic_number = atomic_number - assert atom_state.resolve_chemical_symbol(logger) == chemical_symbol + assert atom_state.resolve_chemical_symbol(logger=logger) == chemical_symbol From b58c9cac90816d2348737448b2a1e74a426f8775 Mon Sep 17 00:00:00 2001 From: JosePizarro3 Date: Wed, 5 Jun 2024 16:43:21 +0200 Subject: [PATCH 04/12] Added coverage in README --- .gitignore | 1 + README.md | 5 ++--- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/.gitignore b/.gitignore index e744d987..65d09cd0 100644 --- a/.gitignore +++ b/.gitignore @@ -45,6 +45,7 @@ htmlcov/ .cache nosetests.xml coverage.xml +coverage.txt *.cover *.py,cover .hypothesis/ diff --git a/README.md b/README.md index 38f7f528..73f5f245 100644 --- a/README.md +++ b/README.md @@ -42,12 +42,11 @@ python -m pytest -sv where the `-s` and `-v` options toggle the output verbosity. -Our CI/CD pipeline produces a more comprehensive test report using `coverage` and `coveralls` packages. -To emulate this locally, perform: +Our CI/CD pipeline produces a more comprehensive test report using `coverage` and `coveralls` packages. We suggest you to generate your own coverage reports locally by doing: ```sh pip install coverage coveralls -python -m coverage run -m pytest -sv +python -m pytest --cov=src ``` ## Development From 55963e898f5672faee778b2189831789b64a5a6b Mon Sep 17 00:00:00 2001 From: JosePizarro3 Date: Wed, 5 Jun 2024 17:52:06 +0200 Subject: [PATCH 05/12] Added testing for model_method.py classes related with TB --- src/nomad_simulations/model_method.py | 118 +++---- tests/test_model_method.py | 478 ++++++++++++++++++++++++++ 2 files changed, 538 insertions(+), 58 deletions(-) create mode 100644 tests/test_model_method.py diff --git a/src/nomad_simulations/model_method.py b/src/nomad_simulations/model_method.py index f2d4ce40..68619892 100644 --- a/src/nomad_simulations/model_method.py +++ b/src/nomad_simulations/model_method.py @@ -31,10 +31,10 @@ Context, ) -from .numerical_settings import NumericalSettings -from .model_system import ModelSystem -from .atoms_state import OrbitalsState, CoreHole -from .utils import is_not_representative +from nomad_simulations.numerical_settings import NumericalSettings +from nomad_simulations.model_system import ModelSystem, AtomicCell +from nomad_simulations.atoms_state import OrbitalsState, CoreHole +from nomad_simulations.utils import is_not_representative class ModelMethod(ArchiveSection): @@ -448,14 +448,11 @@ class TB(ModelMethodElectronic): """, ) - def resolve_type(self, logger: BoundLogger) -> Optional[str]: + def resolve_type(self) -> Optional[str]: """ Resolves the `type` of the `TB` section if it is not already defined, and from the `m_def.name` of the section. - Args: - logger (BoundLogger): The logger to log messages. - Returns: (Optional[str]): The resolved `type` of the `TB` section. """ @@ -482,23 +479,29 @@ def resolve_orbital_references( Returns: Optional[List[OrbitalsState]]: The resolved references to the `OrbitalsState` sections. """ - model_system = model_systems[model_index] + try: + model_system = model_systems[model_index] + except IndexError: + logger.warning( + f'The `ModelSystem` section with index {model_index} was not found.' + ) + return None # If `ModelSystem` is not representative, the normalization will not run - if is_not_representative(model_system, logger): + if is_not_representative(model_system=model_system, logger=logger): return None # If `AtomicCell` is not found, the normalization will not run - atomic_cell = model_system.cell[0] - if atomic_cell is None: + if not model_system.cell: logger.warning('`AtomicCell` section was not found.') return None + atomic_cell = model_system.cell[0] # If there is no child `ModelSystem`, the normalization will not run atoms_state = atomic_cell.atoms_state model_system_child = model_system.model_system - if model_system_child is None: - logger.warning('No child `ModelSystem` section was found.') + if not atoms_state or not model_system_child: + logger.warning('No `AtomsState` or child `ModelSystem` section were found.') return None # We flatten the `OrbitalsState` sections from the `ModelSystem` section @@ -509,7 +512,13 @@ def resolve_orbital_references( continue indices = active_atom.atom_indices for index in indices: - active_atoms_state = atoms_state[index] + try: + active_atoms_state = atoms_state[index] + except IndexError: + logger.warning( + f'The `AtomsState` section with index {index} was not found.' + ) + continue orbitals_state = active_atoms_state.orbitals_state for orbital in orbitals_state: orbitals_ref.append(orbital) @@ -522,11 +531,7 @@ def normalize(self, archive, logger) -> None: self.name = 'TB' # Resolve `type` to be defined by the lower level class (Wannier, DFTB, xTB or SlaterKoster) if it is not already defined - self.type = ( - self.resolve_type(logger) - if (self.type is None or self.type == 'unavailable') - else self.type - ) + self.type = self.resolve_type() # Resolve `orbitals_ref` from the info in the child `ModelSystem` section and the `OrbitalsState` sections model_systems = self.m_xpath('m_parent.model_system', dict=False) @@ -536,8 +541,10 @@ def normalize(self, archive, logger) -> None: ) return # This normalization only considers the last `ModelSystem` (default `model_index` argument set to -1) - orbitals_ref = self.resolve_orbital_references(model_systems, logger) - if orbitals_ref is not None and self.orbitals_ref is None: + orbitals_ref = self.resolve_orbital_references( + model_systems=model_systems, logger=logger + ) + if orbitals_ref is not None and len(orbitals_ref) > 0 and not self.orbitals_ref: self.n_orbitals = len(orbitals_ref) self.orbitals_ref = orbitals_ref @@ -586,32 +593,16 @@ class Wannier(TB): """, ) - def resolve_localization_type(self, logger: BoundLogger) -> Optional[str]: - """ - Resolves the `localization_type` of the `Wannier` section if it is not already defined, and from the - `is_maximally_localized` boolean. - - Args: - logger (BoundLogger): The logger to log messages. - - Returns: - (Optional[str]): The resolved `localization_type` of the `Wannier` section. - """ - if self.localization_type is None: - if self.is_maximally_localized: - return 'maximally_localized' - else: - return 'single_shot' - logger.info( - 'Could not find if the Wannier tight-binding model is maximally localized or not.' - ) - return None - def normalize(self, archive, logger): super().normalize(archive, logger) # Resolve `localization_type` from `is_maximally_localized` - self.localization_type = self.resolve_localization_type(logger) + if self.localization_type is None: + if self.is_maximally_localized is not None: + if self.is_maximally_localized: + self.localization_type = 'maximally_localized' + else: + self.localization_type = 'single_shot' class SlaterKosterBond(ArchiveSection): @@ -680,29 +671,39 @@ def __init__(self, m_def: Section = None, m_context: Context = None, **kwargs): def resolve_bond_name_from_references( self, - orbital_1: OrbitalsState, - orbital_2: OrbitalsState, - bravais_vector: tuple, + orbital_1: Optional[OrbitalsState], + orbital_2: Optional[OrbitalsState], + bravais_vector: Optional[tuple], logger: BoundLogger, ) -> Optional[str]: """ Resolves the `name` of the `SlaterKosterBond` from the references to the `OrbitalsState` sections. Args: - orbital_1 (OrbitalsState): The first `OrbitalsState` section. - orbital_2 (OrbitalsState): The second `OrbitalsState` section. - bravais_vector (tuple): The bravais vector of the cell. + orbital_1 (Optional[OrbitalsState]): The first `OrbitalsState` section. + orbital_2 (Optional[OrbitalsState]): The second `OrbitalsState` section. + bravais_vector (Optional[tuple]): The bravais vector of the cell. logger (BoundLogger): The logger to log messages. Returns: (Optional[str]): The resolved `name` of the `SlaterKosterBond`. """ - bond_name = None + # Initial check + if orbital_1 is None or orbital_2 is None: + logger.warning('The `OrbitalsState` sections are not defined.') + return None + if bravais_vector is None: + logger.warning('The `bravais_vector` is not defined.') + return None + + # Check for `l_quantum_symbol` in `OrbitalsState` sections if orbital_1.l_quantum_symbol is None or orbital_2.l_quantum_symbol is None: logger.warning( 'The `l_quantum_symbol` of the `OrbitalsState` bonds are not defined.' ) return None + + bond_name = None value = [orbital_1.l_quantum_symbol, orbital_2.l_quantum_symbol, bravais_vector] # Check if `value` is found in the `self._bond_name_map` and return the key for key, val in self._bond_name_map.items(): @@ -714,14 +715,15 @@ def resolve_bond_name_from_references( def normalize(self, archive, logger) -> None: super().normalize(archive, logger) + # Resolve the SK bond `name` from the `OrbitalsState` references and the `bravais_vector` if self.orbital_1 and self.orbital_2 and self.bravais_vector is not None: - bravais_vector = tuple(self.bravais_vector) # transformed for comparing - self.name = ( - self.resolve_bond_name_from_references( - self.orbital_1, self.orbital_2, bravais_vector, logger - ) - if self.name is None - else self.name + if self.bravais_vector is not None: + bravais_vector = tuple(self.bravais_vector) # transformed for comparing + self.name = self.resolve_bond_name_from_references( + orbital_1=self.orbital_1, + orbital_2=self.orbital_2, + bravais_vector=bravais_vector, + logger=logger, ) diff --git a/tests/test_model_method.py b/tests/test_model_method.py new file mode 100644 index 00000000..80071673 --- /dev/null +++ b/tests/test_model_method.py @@ -0,0 +1,478 @@ +# +# Copyright The NOMAD Authors. +# +# This file is part of NOMAD. See https://nomad-lab.eu for further info. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +# + +import pytest +import numpy as np +from typing import List, Union, Optional, Tuple + +from nomad.datamodel import EntryArchive + +from nomad_simulations.model_method import TB, Wannier, SlaterKoster, SlaterKosterBond +from nomad_simulations.atoms_state import OrbitalsState, AtomsState +from nomad_simulations import Simulation +from nomad_simulations.model_system import ModelSystem, AtomicCell + +from . import logger +from .conftest import generate_simulation + + +class TestTB: + """ + Test the `TB` class defined in `model_method.py`. + """ + + @pytest.mark.parametrize( + 'tb_section, result', + [(Wannier(), 'Wannier'), (SlaterKoster(), 'SlaterKoster'), (TB(), None)], + ) + def test_resolve_type(self, tb_section: TB, result: Optional[str]): + """ + Test the `resolve_type` method. + + Args: + tb_section (TB): The TB section to resolve the type from. + result (Optional[str]): The expected type of the TB section. + """ + assert tb_section.resolve_type() == result + + @pytest.mark.parametrize( + 'model_systems, model_index, result', + [ + # no `ModelSystem` sections + ([], 0, None), + # `model_index` out of range + ([ModelSystem()], 1, None), + # no `is_representative` in `ModelSystem` + ([ModelSystem(is_representative=False)], 0, None), + # no `cell` section in `ModelSystem` + ([ModelSystem(is_representative=True)], 0, None), + # no `AtomsState` in `AtomicCell` + ([ModelSystem(is_representative=True, cell=[AtomicCell()])], 0, None), + # no `model_system` child section under `ModelSystem` + ( + [ + ModelSystem( + is_representative=True, + cell=[AtomicCell(atoms_state=[AtomsState()])], + ) + ], + 0, + None, + ), + # `type` for the `model_system` child is not `'active_atom'` + ( + [ + ModelSystem( + is_representative=True, + cell=[AtomicCell(atoms_state=[AtomsState()])], + model_system=[ModelSystem(type='bulk')], + ) + ], + 0, + [], + ), + # wrong index for `AtomsState in active atom + ( + [ + ModelSystem( + is_representative=True, + cell=[AtomicCell(atoms_state=[AtomsState()])], + model_system=[ + ModelSystem(type='active_atom', atom_indices=[2]) + ], + ) + ], + 0, + [], + ), + # empty `OrbitalsState` in `AtomsState` + ( + [ + ModelSystem( + is_representative=True, + cell=[AtomicCell(atoms_state=[AtomsState(orbitals_state=[])])], + model_system=[ + ModelSystem(type='active_atom', atom_indices=[0]) + ], + ) + ], + 0, + [], + ), + # valid case + ( + [ + ModelSystem( + is_representative=True, + cell=[ + AtomicCell( + atoms_state=[ + AtomsState( + orbitals_state=[ + OrbitalsState(l_quantum_symbol='s') + ] + ) + ] + ) + ], + model_system=[ + ModelSystem(type='active_atom', atom_indices=[0]) + ], + ) + ], + 0, + [OrbitalsState(l_quantum_symbol='s')], + ), + ], + ) + def test_resolve_orbital_references( + self, + model_systems: Optional[List[ModelSystem]], + model_index: int, + result: Optional[List[OrbitalsState]], + ): + """ + Test the `resolve_orbital_references` method. + + Args: + model_systems (Optional[List[ModelSystem]]): The `model_system` section to add to `Simulation`. + model_index (int): The index of the `ModelSystem` section to resolve the orbital references from. + result (Optional[List[OrbitalsState]]): The expected orbital references. + """ + tb_method = TB() + simulation = generate_simulation(model_method=tb_method) + simulation.model_system = model_systems + orbitals_ref = tb_method.resolve_orbital_references( + model_systems=model_systems, + logger=logger, + model_index=model_index, + ) + if not orbitals_ref: + assert orbitals_ref == result + else: + assert orbitals_ref[0].l_quantum_symbol == result[0].l_quantum_symbol + + @pytest.mark.parametrize( + 'tb_section, result_type, model_systems, result', + [ + # no method `type` extracted + (TB(), 'unavailable', [], None), + # method `type` extracted + (Wannier(), 'Wannier', [], None), + # no `ModelSystem` sections + (Wannier(), 'Wannier', [], None), + # no `is_representative` in `ModelSystem` + (Wannier(), 'Wannier', [ModelSystem(is_representative=False)], None), + # no `cell` section in `ModelSystem` + (Wannier(), 'Wannier', [ModelSystem(is_representative=True)], None), + # no `AtomsState` in `AtomicCell` + ( + Wannier(), + 'Wannier', + [ModelSystem(is_representative=True, cell=[AtomicCell()])], + None, + ), + # no `model_system` child section under `ModelSystem` + ( + Wannier(), + 'Wannier', + [ + ModelSystem( + is_representative=True, + cell=[AtomicCell(atoms_state=[AtomsState()])], + ) + ], + None, + ), + # `type` for the `model_system` child is not `'active_atom'` + ( + Wannier(), + 'Wannier', + [ + ModelSystem( + is_representative=True, + cell=[AtomicCell(atoms_state=[AtomsState()])], + model_system=[ModelSystem(type='bulk')], + ) + ], + None, + ), + # wrong index for `AtomsState in active atom + ( + Wannier(), + 'Wannier', + [ + ModelSystem( + is_representative=True, + cell=[AtomicCell(atoms_state=[AtomsState()])], + model_system=[ + ModelSystem(type='active_atom', atom_indices=[2]) + ], + ) + ], + None, + ), + # empty `OrbitalsState` in `AtomsState` + ( + Wannier(), + 'Wannier', + [ + ModelSystem( + is_representative=True, + cell=[AtomicCell(atoms_state=[AtomsState(orbitals_state=[])])], + model_system=[ + ModelSystem(type='active_atom', atom_indices=[0]) + ], + ) + ], + None, + ), + # `Wannier.orbitals_ref` already set up + ( + Wannier(orbitals_ref=[OrbitalsState(l_quantum_symbol='d')]), + 'Wannier', + [ + ModelSystem( + is_representative=True, + cell=[ + AtomicCell( + atoms_state=[ + AtomsState( + orbitals_state=[ + OrbitalsState(l_quantum_symbol='s') + ] + ) + ] + ) + ], + model_system=[ + ModelSystem(type='active_atom', atom_indices=[0]) + ], + ) + ], + [OrbitalsState(l_quantum_symbol='d')], + ), + # valid case + ( + Wannier(), + 'Wannier', + [ + ModelSystem( + is_representative=True, + cell=[ + AtomicCell( + atoms_state=[ + AtomsState( + orbitals_state=[ + OrbitalsState(l_quantum_symbol='s') + ] + ) + ] + ) + ], + model_system=[ + ModelSystem(type='active_atom', atom_indices=[0]) + ], + ) + ], + [OrbitalsState(l_quantum_symbol='s')], + ), + ], + ) + def test_normalize( + self, + tb_section: TB, + result_type: Optional[str], + model_systems: Optional[List[ModelSystem]], + result: Optional[List[OrbitalsState]], + ): + """ + Test the `resolve_orbital_references` method. + + Args: + tb_section (TB): The TB section to resolve the type from. + result_type (Optional[str]): The expected type of the TB section. + model_systems (Optional[List[ModelSystem]]): The `model_system` section to add to `Simulation`. + result (Optional[List[OrbitalsState]]): The expected orbital references. + """ + simulation = generate_simulation(model_method=tb_section) + simulation.model_system = model_systems + tb_section.normalize(EntryArchive(), logger) + assert tb_section.type == result_type + if tb_section.orbitals_ref is not None: + assert len(tb_section.orbitals_ref) == 1 + assert ( + tb_section.orbitals_ref[0].l_quantum_symbol + == result[0].l_quantum_symbol + ) + else: + assert tb_section.orbitals_ref == result + + +class TestWannier: + """ + Test the `Wannier` class defined in `model_method.py`. + """ + + @pytest.mark.parametrize( + 'localization_type, is_maximally_localized, result_localization_type', + [ + # `localization_type` and `is_maximally_localized` are `None` + (None, None, None), + # `localization_type` set while `is_maximally_localized` is `None` + ('single_shot', None, 'single_shot'), + # normalizing from `is_maximally_localized` + (None, True, 'maximally_localized'), + (None, False, 'single_shot'), + ], + ) + def test_normalize( + self, + localization_type: Optional[str], + is_maximally_localized: bool, + result_localization_type: Optional[str], + ): + """ + Test the `normalize` method . + + Args: + localization_type (Optional[str]): The localization type. + is_maximally_localized (bool): If the localization is maximally-localized or a single-shot. + result_localization_type (Optional[str]): The expected `localization_type` after normalization. + """ + wannier = Wannier( + localization_type=localization_type, + is_maximally_localized=is_maximally_localized, + ) + wannier.normalize(EntryArchive(), logger) + assert wannier.localization_type == result_localization_type + + +class TestSlaterKosterBond: + """ + Test the `SlaterKosterBond` class defined in `model_method.py`. + """ + + @pytest.mark.parametrize( + 'orbital_1, orbital_2, bravais_vector, result', + [ + # no `OrbitalsState` sections + (None, None, (), None), + (None, OrbitalsState(), (), None), + (OrbitalsState(), None, (), None), + # no `bravais_vector` + (OrbitalsState(), OrbitalsState(), None, None), + # no `l_quantum_symbol` in `OrbitalsState` + (OrbitalsState(), OrbitalsState(), (0, 0, 0), None), + # valid cases + ( + OrbitalsState(l_quantum_symbol='s'), + OrbitalsState(l_quantum_symbol='s'), + (0, 0, 0), + 'sss', + ), + ( + OrbitalsState(l_quantum_symbol='s'), + OrbitalsState(l_quantum_symbol='p'), + (0, 0, 0), + 'sps', + ), + ], + ) + def test_resolve_bond_name_from_references( + self, + orbital_1: Optional[OrbitalsState], + orbital_2: Optional[OrbitalsState], + bravais_vector: Optional[tuple], + result: Optional[str], + ): + """ + Test the `resolve_bond_name_from_references` method. + + Args: + orbital_1 (Optional[OrbitalsState]): The first `OrbitalsState` section. + orbital_2 (Optional[OrbitalsState]): The second `OrbitalsState` section. + bravais_vector (Optional[tuple]): The bravais vector. + result (Optional[str]): The expected bond name. + """ + sk_bond = SlaterKosterBond() + bond_name = sk_bond.resolve_bond_name_from_references( + orbital_1=orbital_1, + orbital_2=orbital_2, + bravais_vector=bravais_vector, + logger=logger, + ) + assert bond_name == result + + @pytest.mark.parametrize( + 'orbital_1, orbital_2, bravais_vector, result', + [ + # no `OrbitalsState` sections + (None, None, [], None), + (None, OrbitalsState(), [], None), + (OrbitalsState(), None, [], None), + # no `bravais_vector` + (OrbitalsState(), OrbitalsState(), None, None), + # no `l_quantum_symbol` in `OrbitalsState` + (OrbitalsState(), OrbitalsState(), (0, 0, 0), None), + # valid cases + ( + OrbitalsState(l_quantum_symbol='s'), + OrbitalsState(l_quantum_symbol='s'), + (0, 0, 0), + 'sss', + ), + ( + OrbitalsState(l_quantum_symbol='s'), + OrbitalsState(l_quantum_symbol='p'), + (0, 0, 0), + 'sps', + ), + ], + ) + def test_normalize( + self, + orbital_1: Optional[OrbitalsState], + orbital_2: Optional[OrbitalsState], + bravais_vector: Optional[tuple], + result: Optional[str], + ): + """ + Test the `normalize` method. + + Args: + orbital_1 (Optional[OrbitalsState]): The first `OrbitalsState` section. + orbital_2 (Optional[OrbitalsState]): The second `OrbitalsState` section. + bravais_vector (Optional[tuple]): The bravais vector. + result (Optional[str]): The expected SK bond `name` after normalization. + """ + sk_bond = SlaterKosterBond() + atoms_state = AtomsState() + simulation = Simulation( + model_system=[ModelSystem(cell=[AtomicCell(atoms_state=[atoms_state])])] + ) + if orbital_1 is not None: + atoms_state.orbitals_state.append(orbital_1) + sk_bond.orbital_1 = atoms_state.orbitals_state[0] + if orbital_2 is not None: + atoms_state.orbitals_state.append(orbital_2) + sk_bond.orbital_2 = atoms_state.orbitals_state[-1] + if bravais_vector is not None and len(bravais_vector) != 0: + sk_bond.bravais_vector = bravais_vector + sk_bond.normalize(EntryArchive(), logger) + assert sk_bond.name == result From ab84ca6f7e885212a06bebe81a7acf8cfab180da Mon Sep 17 00:00:00 2001 From: JosePizarro3 Date: Wed, 5 Jun 2024 17:59:58 +0200 Subject: [PATCH 06/12] Added explicitly input in functions for numerical_settings.py and its testing --- src/nomad_simulations/numerical_settings.py | 38 ++++++++++++--------- tests/test_numerical_settings.py | 4 +-- 2 files changed, 23 insertions(+), 19 deletions(-) diff --git a/src/nomad_simulations/numerical_settings.py b/src/nomad_simulations/numerical_settings.py index b28843d5..6157c9b4 100644 --- a/src/nomad_simulations/numerical_settings.py +++ b/src/nomad_simulations/numerical_settings.py @@ -226,7 +226,7 @@ def resolve_high_symmetry_points( lattice = None for model_system in model_systems: # General checks to proceed with normalization - if is_not_representative(model_system, logger): + if is_not_representative(model_system=model_system, logger=logger): continue if model_system.symmetry is None: logger.warning('Could not find `ModelSystem.symmetry`.') @@ -253,9 +253,9 @@ def resolve_high_symmetry_points( ) continue # function defined in AtomicCell - atoms = prim_atomic_cell.to_ase_atoms(logger) + atoms = prim_atomic_cell.to_ase_atoms(logger=logger) cell = atoms.get_cell() - lattice = cell.get_bravais_lattice(eps) + lattice = cell.get_bravais_lattice(eps=eps) break # only cover the first representative `ModelSystem` # Checking if `bravais_lattice` and `lattice` are defined @@ -380,8 +380,8 @@ def resolve_points_and_offset( offset = np.array([0, 0, 0]) elif self.center == 'Monkhorst-Pack': try: - points = monkhorst_pack(self.grid) - offset = get_monkhorst_pack_size_and_offset(points)[-1] + points = monkhorst_pack(size=self.grid) + offset = get_monkhorst_pack_size_and_offset(kpts=points)[-1] except ValueError: logger.warning( 'Could not resolve `KMesh.points` and `KMesh.offset` from `KMesh.grid`. ASE `monkhorst_pack` failed.' @@ -448,7 +448,7 @@ def resolve_k_line_density( for model_system in model_systems: # General checks to proceed with normalization - if is_not_representative(model_system, logger): + if is_not_representative(model_system=model_system, logger=logger): continue # TODO extend this for other dimensions (@ndaelman-hu) if model_system.type != 'bulk': @@ -457,7 +457,7 @@ def resolve_k_line_density( # Resolve `k_line_density` if k_line_density := self.get_k_line_density( - reciprocal_lattice_vectors, logger + reciprocal_lattice_vectors=reciprocal_lattice_vectors, logger=logger ): return k_line_density return None @@ -472,7 +472,7 @@ def normalize(self, archive, logger) -> None: # Normalize k mesh from grid sampling if self.points is None and self.offset is None: - self.points, self.offset = self.resolve_points_and_offset(logger) + self.points, self.offset = self.resolve_points_and_offset(logger=logger) # Calculate k_line_density for data quality measures model_systems = self.m_xpath( @@ -483,7 +483,9 @@ def normalize(self, archive, logger) -> None: ) if self.k_line_density is None: self.k_line_density = self.resolve_k_line_density( - model_systems, reciprocal_lattice_vectors, logger + model_systems=model_systems, + reciprocal_lattice_vectors=reciprocal_lattice_vectors, + logger=logger, ) # Resolve `high_symmetry_points` @@ -624,7 +626,7 @@ def get_high_symmetry_path_norms( `high_symmetry_path_value_norms = [0, 0.5, 0.5 + 1 / np.sqrt(2), 1 + 1 / np.sqrt(2)]` """ # Checking the high symmetry path quantities - if not self.validate_high_symmetry_path(logger): + if not self.validate_high_symmetry_path(logger=logger): return None # Checking if `reciprocal_lattice_vectors` is defined and taking its magnitude to operate if reciprocal_lattice_vectors is None: @@ -672,7 +674,7 @@ def resolve_points( logger (BoundLogger): The logger to log messages. """ # General checks for quantities - if not self.validate_high_symmetry_path(logger): + if not self.validate_high_symmetry_path(logger=logger): return None if reciprocal_lattice_vectors is None: logger.warning( @@ -693,7 +695,7 @@ def resolve_points( # Calculate the norms in the path and find the closest indices in points_norm to the high symmetry path norms high_symmetry_path_value_norms = self.get_high_symmetry_path_norms( - reciprocal_lattice_vectors, logger + reciprocal_lattice_vectors=reciprocal_lattice_vectors, logger=logger ) closest_indices = list( map( @@ -751,11 +753,13 @@ def normalize(self, archive, logger) -> None: or len(self.high_symmetry_path_values) == 0 ): self.high_symmetry_path_values = self.resolve_high_symmetry_path_values( - model_systems, reciprocal_lattice_vectors, logger + model_systems=model_systems, + reciprocal_lattice_vectors=reciprocal_lattice_vectors, + logger=logger, ) # If `high_symmetry_path` is not defined, we do not normalize the KLinePath - if not self.validate_high_symmetry_path(logger): + if not self.validate_high_symmetry_path(logger=logger): return @@ -801,7 +805,7 @@ def resolve_reciprocal_lattice_vectors( """ for model_system in model_systems: # General checks to proceed with normalization - if is_not_representative(model_system, logger): + if is_not_representative(model_system=model_system, logger=logger): continue # TODO extend this for other dimensions (@ndaelman-hu) @@ -815,7 +819,7 @@ def resolve_reciprocal_lattice_vectors( continue # Set the `reciprocal_lattice_vectors` using ASE - ase_atoms = atomic_cell[0].to_ase_atoms(logger) + ase_atoms = atomic_cell[0].to_ase_atoms(logger=logger) return 2 * np.pi * ase_atoms.get_reciprocal_cell() / ureg.angstrom return None @@ -826,7 +830,7 @@ def normalize(self, archive, logger) -> None: model_systems = self.m_xpath('m_parent.m_parent.model_system', dict=False) if self.reciprocal_lattice_vectors is None: self.reciprocal_lattice_vectors = self.resolve_reciprocal_lattice_vectors( - model_systems, logger + model_systems=model_systems, logger=logger ) diff --git a/tests/test_numerical_settings.py b/tests/test_numerical_settings.py index 9ef5753a..242c0f08 100644 --- a/tests/test_numerical_settings.py +++ b/tests/test_numerical_settings.py @@ -182,7 +182,7 @@ def test_resolve_points_and_offset( k_mesh = KMesh(center=center) if grid is not None: k_mesh.grid = grid - points, offset = k_mesh.resolve_points_and_offset(logger) + points, offset = k_mesh.resolve_points_and_offset(logger=logger) if points is not None: assert np.allclose(points, result_points) else: @@ -305,7 +305,7 @@ def test_validate_high_symmetry_path( high_symmetry_path_names=high_symmetry_path_names, high_symmetry_path_values=high_symmetry_path_values, ) - assert k_line_path.validate_high_symmetry_path(logger) == result + assert k_line_path.validate_high_symmetry_path(logger=logger) == result @pytest.mark.parametrize( 'reciprocal_lattice_vectors, high_symmetry_path_names, result', From d5282f770a20bd118ae7de6606610b0cbb322d7a Mon Sep 17 00:00:00 2001 From: JosePizarro3 Date: Wed, 5 Jun 2024 18:01:52 +0200 Subject: [PATCH 07/12] Fix imports using abspath to package --- src/nomad_simulations/outputs.py | 8 ++++---- src/nomad_simulations/physical_property.py | 4 ++-- src/nomad_simulations/properties/band_gap.py | 2 +- src/nomad_simulations/properties/energies.py | 2 +- src/nomad_simulations/properties/hopping_matrix.py | 2 +- src/nomad_simulations/properties/spectral_profile.py | 10 +++++----- 6 files changed, 14 insertions(+), 14 deletions(-) diff --git a/src/nomad_simulations/outputs.py b/src/nomad_simulations/outputs.py index 07e26b40..388e6504 100644 --- a/src/nomad_simulations/outputs.py +++ b/src/nomad_simulations/outputs.py @@ -23,10 +23,10 @@ from nomad.metainfo import Quantity, SubSection from nomad.datamodel.metainfo.annotations import ELNAnnotation -from .model_system import ModelSystem -from .physical_property import PhysicalProperty -from .numerical_settings import SelfConsistency -from .properties import ( +from nomad_simulations.model_system import ModelSystem +from nomad_simulations.physical_property import PhysicalProperty +from nomad_simulations.numerical_settings import SelfConsistency +from nomad_simulations.properties import ( FermiLevel, ChemicalPotential, CrystalFieldSplitting, diff --git a/src/nomad_simulations/physical_property.py b/src/nomad_simulations/physical_property.py index 0f908ac4..39c1d926 100644 --- a/src/nomad_simulations/physical_property.py +++ b/src/nomad_simulations/physical_property.py @@ -35,8 +35,8 @@ from nomad.metainfo.metainfo import DirectQuantity, Dimension, _placeholder_quantity from nomad.datamodel.metainfo.basesections import Entity -from .variables import Variables -from .numerical_settings import SelfConsistency +from nomad_simulations.variables import Variables +from nomad_simulations.numerical_settings import SelfConsistency # We add `logger` for the `PhysicalProperty.variables_shape` method diff --git a/src/nomad_simulations/properties/band_gap.py b/src/nomad_simulations/properties/band_gap.py index 0656197a..6245bc63 100644 --- a/src/nomad_simulations/properties/band_gap.py +++ b/src/nomad_simulations/properties/band_gap.py @@ -23,7 +23,7 @@ from nomad.metainfo import Quantity, MEnum, Section, Context -from ..physical_property import PhysicalProperty +from nomad_simulations.physical_property import PhysicalProperty class ElectronicBandGap(PhysicalProperty): diff --git a/src/nomad_simulations/properties/energies.py b/src/nomad_simulations/properties/energies.py index 8d8209a3..3283a1f8 100644 --- a/src/nomad_simulations/properties/energies.py +++ b/src/nomad_simulations/properties/energies.py @@ -20,7 +20,7 @@ from nomad.metainfo import Quantity, Section, Context -from ..physical_property import PhysicalProperty +from nomad_simulations.physical_property import PhysicalProperty class FermiLevel(PhysicalProperty): diff --git a/src/nomad_simulations/properties/hopping_matrix.py b/src/nomad_simulations/properties/hopping_matrix.py index 7eb6713d..57652eb0 100644 --- a/src/nomad_simulations/properties/hopping_matrix.py +++ b/src/nomad_simulations/properties/hopping_matrix.py @@ -20,7 +20,7 @@ from nomad.metainfo import Quantity, Section, Context -from ..physical_property import PhysicalProperty +from nomad_simulations.physical_property import PhysicalProperty class HoppingMatrix(PhysicalProperty): diff --git a/src/nomad_simulations/properties/spectral_profile.py b/src/nomad_simulations/properties/spectral_profile.py index ddfcfaa6..9b11d94e 100644 --- a/src/nomad_simulations/properties/spectral_profile.py +++ b/src/nomad_simulations/properties/spectral_profile.py @@ -24,11 +24,11 @@ from nomad import config from nomad.metainfo import Quantity, SubSection, Section, Context, MEnum -from ..utils import get_sibling_section, get_variables -from ..physical_property import PhysicalProperty -from ..variables import Energy2 as Energy -from ..atoms_state import AtomsState, OrbitalsState -from .band_gap import ElectronicBandGap +from nomad_simulations.utils import get_sibling_section, get_variables +from nomad_simulations.physical_property import PhysicalProperty +from nomad_simulations.variables import Energy2 as Energy +from nomad_simulations.atoms_state import AtomsState, OrbitalsState +from nomad_simulations.properties.band_gap import ElectronicBandGap class SpectralProfile(PhysicalProperty): From 3f4308ac16574dcbf0cf5981eb744259fdbce910 Mon Sep 17 00:00:00 2001 From: JosePizarro3 Date: Wed, 5 Jun 2024 18:10:10 +0200 Subject: [PATCH 08/12] Added sh script to generate coverage.txt in the project --- .gitignore | 1 - coverage.txt | 55 ++++++++++++++++++++++++++++++++ scripts/generate_coverage_txt.sh | 9 ++++++ 3 files changed, 64 insertions(+), 1 deletion(-) create mode 100644 coverage.txt create mode 100755 scripts/generate_coverage_txt.sh diff --git a/.gitignore b/.gitignore index 65d09cd0..e744d987 100644 --- a/.gitignore +++ b/.gitignore @@ -45,7 +45,6 @@ htmlcov/ .cache nosetests.xml coverage.xml -coverage.txt *.cover *.py,cover .hypothesis/ diff --git a/coverage.txt b/coverage.txt new file mode 100644 index 00000000..c4293a71 --- /dev/null +++ b/coverage.txt @@ -0,0 +1,55 @@ +============================= test session starts ============================== +platform linux -- Python 3.9.19, pytest-5.4.3, py-1.11.0, pluggy-0.13.1 +rootdir: /home/josepizarro/nomad, inifile: pytest.ini +plugins: xdist-1.34.0, timeout-1.4.2, anyio-4.3.0, devtools-0.12.2, forked-1.6.0, cov-2.7.1 +collected 299 items + +tests/test_atoms_state.py ............................................. [ 15%] +tests/test_band_gap.py ..................... [ 22%] +tests/test_band_structure.py ................................ [ 32%] +tests/test_energies.py .. [ 33%] +tests/test_fermi_surface.py .. [ 34%] +tests/test_general.py .......... [ 37%] +tests/test_hopping_matrix.py .... [ 38%] +tests/test_model_method.py ........................................... [ 53%] +tests/test_model_system.py ........................ [ 61%] +tests/test_numerical_settings.py ................................ [ 71%] +tests/test_outputs.py ......................... [ 80%] +tests/test_permittivity.py .......... [ 83%] +tests/test_physical_properties.py ................... [ 89%] +tests/test_spectral_profile.py .................. [ 95%] +tests/test_utils.py ....... [ 98%] +tests/test_variables.py ..... [100%] + +---------- coverage: platform linux, python 3.9.19-final-0 ----------- +Name Stmts Miss Cover +-------------------------------------------------------------------------- +src/nomad_simulations/__init__.py 1 0 100% +src/nomad_simulations/atoms_state.py 187 18 90% +src/nomad_simulations/general.py 68 5 93% +src/nomad_simulations/model_method.py 259 74 71% +src/nomad_simulations/model_system.py 257 19 93% +src/nomad_simulations/numerical_settings.py 260 62 76% +src/nomad_simulations/outputs.py 89 6 93% +src/nomad_simulations/physical_property.py 90 1 99% +src/nomad_simulations/properties/__init__.py 7 0 100% +src/nomad_simulations/properties/band_gap.py 48 2 96% +src/nomad_simulations/properties/band_structure.py 107 20 81% +src/nomad_simulations/properties/energies.py 21 2 90% +src/nomad_simulations/properties/fermi_surface.py 12 1 92% +src/nomad_simulations/properties/hopping_matrix.py 24 2 92% +src/nomad_simulations/properties/permittivity.py 45 5 89% +src/nomad_simulations/properties/spectral_profile.py 254 123 52% +src/nomad_simulations/utils/__init__.py 1 0 100% +src/nomad_simulations/utils/utils.py 66 11 83% +src/nomad_simulations/variables.py 61 8 87% +-------------------------------------------------------------------------- +TOTAL 1857 359 81% + + +============================= 299 passed in 2.18s ============================== + + + + +Generated using './scripts/generate_coverage_txt.sh' in the terminal in the root folder of the project diff --git a/scripts/generate_coverage_txt.sh b/scripts/generate_coverage_txt.sh new file mode 100755 index 00000000..7ae97f91 --- /dev/null +++ b/scripts/generate_coverage_txt.sh @@ -0,0 +1,9 @@ +#!/bin/bash + +# Run pytest with coverage +python -m pytest --cov=src | tee coverage.txt + +# Append the generation message +echo " " >> coverage.txt +echo " " >> coverage.txt +echo -e "\n\nGenerated using './scripts/generate_coverage_txt.sh' in the terminal in the root folder of the project" >> coverage.txt From a83fbdd721d9fe2d92e8aa266550c78cda22312c Mon Sep 17 00:00:00 2001 From: JosePizarro3 Date: Thu, 6 Jun 2024 11:13:08 +0200 Subject: [PATCH 09/12] Deleted coverage.txt file and add it to gitignore --- .gitignore | 1 + coverage.txt | 55 ---------------------------------------------------- 2 files changed, 1 insertion(+), 55 deletions(-) delete mode 100644 coverage.txt diff --git a/.gitignore b/.gitignore index e744d987..65d09cd0 100644 --- a/.gitignore +++ b/.gitignore @@ -45,6 +45,7 @@ htmlcov/ .cache nosetests.xml coverage.xml +coverage.txt *.cover *.py,cover .hypothesis/ diff --git a/coverage.txt b/coverage.txt deleted file mode 100644 index c4293a71..00000000 --- a/coverage.txt +++ /dev/null @@ -1,55 +0,0 @@ -============================= test session starts ============================== -platform linux -- Python 3.9.19, pytest-5.4.3, py-1.11.0, pluggy-0.13.1 -rootdir: /home/josepizarro/nomad, inifile: pytest.ini -plugins: xdist-1.34.0, timeout-1.4.2, anyio-4.3.0, devtools-0.12.2, forked-1.6.0, cov-2.7.1 -collected 299 items - -tests/test_atoms_state.py ............................................. [ 15%] -tests/test_band_gap.py ..................... [ 22%] -tests/test_band_structure.py ................................ [ 32%] -tests/test_energies.py .. [ 33%] -tests/test_fermi_surface.py .. [ 34%] -tests/test_general.py .......... [ 37%] -tests/test_hopping_matrix.py .... [ 38%] -tests/test_model_method.py ........................................... [ 53%] -tests/test_model_system.py ........................ [ 61%] -tests/test_numerical_settings.py ................................ [ 71%] -tests/test_outputs.py ......................... [ 80%] -tests/test_permittivity.py .......... [ 83%] -tests/test_physical_properties.py ................... [ 89%] -tests/test_spectral_profile.py .................. [ 95%] -tests/test_utils.py ....... [ 98%] -tests/test_variables.py ..... [100%] - ----------- coverage: platform linux, python 3.9.19-final-0 ----------- -Name Stmts Miss Cover --------------------------------------------------------------------------- -src/nomad_simulations/__init__.py 1 0 100% -src/nomad_simulations/atoms_state.py 187 18 90% -src/nomad_simulations/general.py 68 5 93% -src/nomad_simulations/model_method.py 259 74 71% -src/nomad_simulations/model_system.py 257 19 93% -src/nomad_simulations/numerical_settings.py 260 62 76% -src/nomad_simulations/outputs.py 89 6 93% -src/nomad_simulations/physical_property.py 90 1 99% -src/nomad_simulations/properties/__init__.py 7 0 100% -src/nomad_simulations/properties/band_gap.py 48 2 96% -src/nomad_simulations/properties/band_structure.py 107 20 81% -src/nomad_simulations/properties/energies.py 21 2 90% -src/nomad_simulations/properties/fermi_surface.py 12 1 92% -src/nomad_simulations/properties/hopping_matrix.py 24 2 92% -src/nomad_simulations/properties/permittivity.py 45 5 89% -src/nomad_simulations/properties/spectral_profile.py 254 123 52% -src/nomad_simulations/utils/__init__.py 1 0 100% -src/nomad_simulations/utils/utils.py 66 11 83% -src/nomad_simulations/variables.py 61 8 87% --------------------------------------------------------------------------- -TOTAL 1857 359 81% - - -============================= 299 passed in 2.18s ============================== - - - - -Generated using './scripts/generate_coverage_txt.sh' in the terminal in the root folder of the project From c0e6553b26ccd432b5b586e5fdedae22c7a8e259 Mon Sep 17 00:00:00 2001 From: JosePizarro3 Date: Thu, 6 Jun 2024 11:14:59 +0200 Subject: [PATCH 10/12] Added Ahmed changes to the pipeline for comment on pr from coverage --- .github/workflows/actions.yaml | 77 ++++++++++++++++++---------------- pyproject.toml | 6 +-- 2 files changed, 45 insertions(+), 38 deletions(-) diff --git a/.github/workflows/actions.yaml b/.github/workflows/actions.yaml index 37849864..83efb9a0 100644 --- a/.github/workflows/actions.yaml +++ b/.github/workflows/actions.yaml @@ -1,47 +1,54 @@ name: install-and-test on: [push] + +# https://docs.github.com/en/actions/using-jobs/assigning-permissions-to-jobs +# `contents` is for permission to the contents of the repository. +# `pull-requests` is for permission to pull request +permissions: + contents: write + checks: write + pull-requests: write + jobs: install-and-test: runs-on: ubuntu-latest steps: - - uses: actions/checkout@v3 - - name: Set up Python 3.9 - uses: actions/setup-python@v2 - with: - python-version: 3.9 - - name: Install dependencies - run: | - pip install --upgrade pip - pip install '.[dev]' --index-url https://gitlab.mpcdf.mpg.de/api/v4/projects/2187/packages/pypi/simple - pip install coverage coveralls - - name: mypy - run: | - python -m mypy --ignore-missing-imports --follow-imports=silent --no-strict-optional src/nomad_simulations tests - - name: Test with pytest - run: | - python -m coverage run -m pytest -sv tests - - name: Submit to coveralls - continue-on-error: true - env: - GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} - run: | - coveralls --service=github + - uses: actions/checkout@v3 + - name: Set up Python 3.9 + uses: actions/setup-python@v2 + with: + python-version: 3.9 + - name: Install dependencies + run: | + pip install uv + uv pip install -e '.[dev]' --index-url https://gitlab.mpcdf.mpg.de/api/v4/projects/2187/packages/pypi/simple --system + - name: mypy + run: | + python -m mypy --ignore-missing-imports --follow-imports=silent --no-strict-optional src/nomad_simulations tests + - name: Build coverage file + run: | + pytest --junitxml=pytest.xml --cov-report=term-missing:skip-covered --cov=src tests/ | tee pytest-coverage.txt + - name: Pytest coverage comment + uses: MishaKav/pytest-coverage-comment@main + with: + pytest-coverage-path: pytest-coverage.txt + junitxml-path: pytest.xml build-and-install: runs-on: ubuntu-latest steps: - - uses: actions/checkout@v3 - - name: Set up Python 3.9 - uses: actions/setup-python@v2 - with: - python-version: 3.9 - - name: Build the package - run: | - pip install --upgrade pip - pip install build - python -m build --sdist - - name: Install the package - run: | - pip install dist/*.tar.gz --index-url https://gitlab.mpcdf.mpg.de/api/v4/projects/2187/packages/pypi/simple + - uses: actions/checkout@v3 + - name: Set up Python 3.9 + uses: actions/setup-python@v2 + with: + python-version: 3.9 + - name: Build the package + run: | + pip install --upgrade pip + pip install build + python -m build --sdist + - name: Install the package + run: | + pip install dist/*.tar.gz --index-url https://gitlab.mpcdf.mpg.de/api/v4/projects/2187/packages/pypi/simple ruff-linting: runs-on: ubuntu-latest steps: diff --git a/pyproject.toml b/pyproject.toml index d09f6427..3a596817 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -31,9 +31,9 @@ dependencies = [ [project.optional-dependencies] dev = [ 'mypy==1.0.1', - 'pytest==3.10.0', - 'pytest-timeout==1.4.2', - 'pytest-cov==2.7.1', + 'pytest', + 'pytest-timeout', + 'pytest-cov', 'ruff', "structlog==22.3.0", "lxml_html_clean>=0.1.0", From 57be7241e025f2946d4bc2fd270ebd177c3ce3a1 Mon Sep 17 00:00:00 2001 From: JosePizarro3 Date: Thu, 6 Jun 2024 13:46:18 +0200 Subject: [PATCH 11/12] Fixed README with more info on the script, uv --- README.md | 18 ++++++++++++++---- scripts/generate_coverage_txt.sh | 4 +--- 2 files changed, 15 insertions(+), 7 deletions(-) diff --git a/README.md b/README.md index 73f5f245..c9595e81 100644 --- a/README.md +++ b/README.md @@ -21,11 +21,16 @@ python3.9 -m venv .pyenv . .pyenv/bin/activate ``` +We recommend installing `uv` for fast pip installation of the packages: +```sh +pip install uv +``` + Install the `nomad-lab` package: ```sh pip install --upgrade pip -pip install '.[dev]' --index-url https://gitlab.mpcdf.mpg.de/api/v4/projects/2187/packages/pypi/simple +uv pip install '.[dev]' --index-url https://gitlab.mpcdf.mpg.de/api/v4/projects/2187/packages/pypi/simple ``` **Note!** @@ -37,7 +42,7 @@ sure to include NOMAD's internal package registry (via `--index-url` in the abov You can run local tests using the `pytest` package: ```sh -python -m pytest -sv +python -m pytest -sv tests ``` where the `-s` and `-v` options toggle the output verbosity. @@ -46,7 +51,12 @@ Our CI/CD pipeline produces a more comprehensive test report using `coverage` an ```sh pip install coverage coveralls -python -m pytest --cov=src +python -m pytest --cov=src tests +``` + +You can also run the script to generate a local file `coverage.txt` with the same information by doing: +```sh +./scripts/generate_coverage_txt.sh ``` ## Development @@ -54,7 +64,7 @@ python -m pytest --cov=src The plugin is still under development. If you would like to contribute, install the package in editable mode (with the added `-e` flag) with the development dependencies: ```sh -pip install -e .[dev] --index-url https://gitlab.mpcdf.mpg.de/api/v4/projects/2187/packages/pypi/simple +uv pip install -e .[dev] --index-url https://gitlab.mpcdf.mpg.de/api/v4/projects/2187/packages/pypi/simple ``` ### Setting up plugin on your local installation diff --git a/scripts/generate_coverage_txt.sh b/scripts/generate_coverage_txt.sh index 7ae97f91..6168ecfc 100755 --- a/scripts/generate_coverage_txt.sh +++ b/scripts/generate_coverage_txt.sh @@ -4,6 +4,4 @@ python -m pytest --cov=src | tee coverage.txt # Append the generation message -echo " " >> coverage.txt -echo " " >> coverage.txt -echo -e "\n\nGenerated using './scripts/generate_coverage_txt.sh' in the terminal in the root folder of the project" >> coverage.txt +echo -e "\n\n\nGenerated using './scripts/generate_coverage_txt.sh' in the terminal in the root folder of the project" >> coverage.txt From 47420ac7278eea60a62d300fa1be76c68e4dc7cc Mon Sep 17 00:00:00 2001 From: JosePizarro3 Date: Thu, 6 Jun 2024 14:23:51 +0200 Subject: [PATCH 12/12] Added uv to pip install in pipeline --- .github/workflows/actions.yaml | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/.github/workflows/actions.yaml b/.github/workflows/actions.yaml index 83efb9a0..8fc223ba 100644 --- a/.github/workflows/actions.yaml +++ b/.github/workflows/actions.yaml @@ -27,7 +27,7 @@ jobs: python -m mypy --ignore-missing-imports --follow-imports=silent --no-strict-optional src/nomad_simulations tests - name: Build coverage file run: | - pytest --junitxml=pytest.xml --cov-report=term-missing:skip-covered --cov=src tests/ | tee pytest-coverage.txt + pytest --junitxml=pytest.xml --cov-report=term-missing:skip-covered --cov=src tests | tee pytest-coverage.txt - name: Pytest coverage comment uses: MishaKav/pytest-coverage-comment@main with: @@ -43,12 +43,13 @@ jobs: python-version: 3.9 - name: Build the package run: | - pip install --upgrade pip - pip install build + pip install uv + uv pip install --upgrade pip --system + uv pip install build --system python -m build --sdist - name: Install the package run: | - pip install dist/*.tar.gz --index-url https://gitlab.mpcdf.mpg.de/api/v4/projects/2187/packages/pypi/simple + uv pip install dist/*.tar.gz --index-url https://gitlab.mpcdf.mpg.de/api/v4/projects/2187/packages/pypi/simple --system ruff-linting: runs-on: ubuntu-latest steps: