Abinit workflows: static, non-scf and relaxation. #183

davidwaroquiers · 2022-10-18T10:03:49Z

Summary

Include a summary of major changes in bullet points:

Scf and non-Scf jobs for abinit
Band Structure flow with abinit
Relaxation for abinit
Supports restarts of scf, non-scf and relax jobs
Supports restarts from a different previous job

Additional dependencies introduced (if any)

abipy

TODO

other flows
add autoparal
uniformize with vasp flows (e.g. prev_vasp_dir vs prev_dirs or prev_outputs or ..., band structures, static maker so that it can be used interchangeabily in the elastic flow, ...) => discuss this with @utf @computron Guido Petretto (@gpetretto), ... who else ?

davidwaroquiers · 2022-10-18T11:01:05Z

Hi @utf and @gpetretto, here is the new PR for abinit (previous discussions available here: #81 ). I think now things are fine. I disabled "Allow edits and access to secrets by maintainers", as I think this is the reason why things went sideways. I do not see how to "request" a review above so I'm leaving it this way.

codecov · 2022-10-18T12:06:01Z

Codecov Report

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

Project coverage is 74.53%. Comparing base (256b39a) to head (66dc14a).
Report is 2 commits behind head on main.

❗ Current head 66dc14a differs from pull request most recent head a27caaf. Consider uploading reports for the commit a27caaf to get more accurate results

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #183      +/-   ##
==========================================
- Coverage   77.03%   74.53%   -2.50%     
==========================================
  Files         122      128       +6     
  Lines        9108     9888     +780     
  Branches     1429     1493      +64     
==========================================
+ Hits         7016     7370     +354     
- Misses       1663     2072     +409     
- Partials      429      446      +17

Files	Coverage Δ
src/atomate2/abinit/utils/__init__.py	`100.00% <100.00%> (ø)`
src/atomate2/settings.py	`100.00% <100.00%> (ø)`
src/atomate2/abinit/powerups.py	`92.30% <92.30%> (ø)`
src/atomate2/common/files.py	`80.39% <33.33%> (-1.61%)`	⬇️
src/atomate2/utils/file_client.py	`43.84% <50.00%> (+0.31%)`	⬆️
src/atomate2/abinit/flows/core.py	`85.36% <85.36%> (ø)`
src/atomate2/abinit/utils/settings.py	`0.00% <0.00%> (ø)`
src/atomate2/abinit/jobs/core.py	`88.52% <88.52%> (ø)`
src/atomate2/abinit/schemas/calculation.py	`90.58% <90.58%> (ø)`
src/atomate2/abinit/files.py	`72.72% <72.72%> (ø)`
... and 8 more

... and 16 files with indirect coverage changes

gpetretto

Here are some more comments. I suppose that some of them could be addressed in a future PR.

src/atomate2/abinit/jobs/base.py

gpetretto · 2022-10-19T08:46:27Z

src/atomate2/abinit/jobs/base.py

+        # Deal with extra_abivars.
+        # First take extra_abivars from the input_set_generator of the current maker.
+        # Update with those from the input_set_generator of the previous maker.
+        # Update with the extra_abivars from the **params if provided.


This seems a point where it would be very easy to get the priority order wrong for a user. If I pass an explicit input_set_generator and I have set extra_abivars there, I probably would not expect them to be overwritten.
Are all the options needed?

You mean that you would remove the last option ? (i.e. possibility to update extra_abivars in the **params)

src/atomate2/abinit/jobs/base.py

gpetretto · 2022-10-20T09:56:42Z

src/atomate2/abinit/schemas/core.py

+    """Definition of task document about an Abinit Job."""
+
+    state: Status = Field(None, description="State of this job.")
+    run_number: int = Field(None, description="Run number of this job.")


could this be replaced by the job index?

Is this still needed? Is this not contained in the history? On that note, do you want to store the history in the task document.

src/atomate2/cli/dev.py

tests/abinit/jobs/test_base.py

gpetretto · 2022-10-21T07:09:46Z

src/atomate2/abinit/schemas/core.py

+    energy: float = Field(None, description="Final energy of the calculation.")
+
+
+class AbinitTaskDocument(StructureMetadata):


No ouputs stored here? Will we revise this later?

Yes, related to #145

Would it be possible to store some outputs here so these workflows could be used? If you follow the VASP task document roughly that would mean it could be a reasonable template to use for a general task document.

Just bumping the above comment.

src/atomate2/abinit/schemas/core.py

utf · 2023-03-21T14:22:39Z

Hi @davidwaroquiers @gpetretto

What is the current status/plan for this PR?

davidwaroquiers · 2023-03-21T14:44:00Z

On my side you can review it. I would favor pushing it asap, pointing out that the abinit workflows are still in experimental development and make smaller PRs afterwards. We can open a series of issues related to this PR and deal with it in subsequent PRs. What do you think?

utf · 2023-03-21T18:09:48Z

My biggest concern with the current PR is how the input sets work. They are quite complicated and I wonder if they could be simplified quite a bit. The goal for the new VASP input sets in atomate2 was to make it so that:

All inputs are configured in the same way.
It is obvious how different input sets differ from each other (you should be able to quickly scan and see what the changes are).
All the logic for generating the inputs is in the base input set so technically any input set can use features that all the other input sets use (e.g., k-point generation schemes, copying files, dependencies)
The fields on the generator classes don't correspond directly to input set parameters themselves, but are more helper functions to set a number of parameters simultaneously (e.g., a band structure mode flag that changes a number of settings related to band structures). The preference is for fewer fields on the generator classes themselves because by their nature they aren't shared between other inputset generators.

With this in mind, I think the input set generators in this PR could be simplified by:

Setting the input parameters using only the extra_abivars dict rather than dataclass fields. This would remove the complicated logic from the base input set maker that extracts certain inputset fields that are abinit settings vs generator settings. The issue with the current approach is that say you want to add a new field that controls something like the band structure mode which doesn't correspond directly to an abinit input settings. Then you will need to add it to the list of protected field names to ignore when extracting the abinit settings etc.
Standardise the get_abinit_input function across all input set generators. Currently, the different generator implementations (e.g., StaticSetGenerator vs NonSCFSetGenerator) have different options specified in this function which makes using the input sets quite confusing.
Because you are using the factory methods from abipy, I wonder whether the factory could be specified as a field on the base input set generator. And then used automatically in the get_input_set without needing to do all of this in get_abinit_input.
Rename extra_abivars to something like factory_kwargs, just to make it clearer what this does.
Add an additional user_abinit_settings to override specific abinit input set settings.
Don't subclass input set implementations. We moved away from this because traversing the hierarchy makes it very difficult to understand why a certain setting is getting set. Instead there are only two levels: 1. the base input set generator; 2. the input set implementation. This might result in a little duplication but I think this is ok if it makes things more obvious for the user.

To give you an example of what I mean, the current StaticSetGenerator looks like this:

@dataclass
class GroundStateSetGenerator(AbinitInputGenerator):
    """Common class for ground-state generators."""

    calc_type: str = "ground_state"

    kppa: Optional[float] = None
    ecut: Optional[float] = None
    pawecutdg: Optional[float] = None
    nband: Optional[int] = None
    accuracy: str = "normal"
    spin_mode: str = "polarized"
    smearing: str = "fermi_dirac:0.1 eV"
    charge: float = 0.0
    scf_algorithm: Optional[str] = None
    shifts: Union[str, tuple] = "Monkhorst-Pack"

    restart_from_deps: tuple = (f"{SCF}|{RELAX}|{MOLECULAR_DYNAMICS}:WFK|DEN",)

@dataclass
class StaticSetGenerator(GroundStateSetGenerator):
    """Class to generate Abinit static input sets."""

    calc_type: str = "static"

    def get_abinit_input(
        self,
        structure=None,
        pseudos=None,
        prev_outputs=None,
        kppa=GroundStateSetGenerator.kppa,
        ecut=GroundStateSetGenerator.ecut,
        pawecutdg=GroundStateSetGenerator.pawecutdg,
        nband=GroundStateSetGenerator.nband,
        accuracy=GroundStateSetGenerator.accuracy,
        spin_mode=GroundStateSetGenerator.spin_mode,
        smearing=GroundStateSetGenerator.smearing,
        charge=GroundStateSetGenerator.charge,
        scf_algorithm=GroundStateSetGenerator.scf_algorithm,
        shifts=GroundStateSetGenerator.shifts,
    ):
        """Get AbinitInput object for static calculation."""
        if structure is None:
            raise RuntimeError("Structure is mandatory for StaticSet generation.")
        if prev_outputs is not None:
            raise RuntimeError(
                "Previous outputs not allowed for StaticSetGenerator. "
                "To restart from a previous static or otherwise scf "
                "(e.g. relaxation) calculation, use restart_from argument of "
                "get_input_set method instead."
            )
        inp = scf_input(
            structure=structure,
            pseudos=pseudos,
            kppa=kppa,
            ecut=ecut,
            pawecutdg=pawecutdg,
            nband=nband,
            accuracy=accuracy,
            spin_mode=spin_mode,
            smearing=smearing,
            charge=charge,
            scf_algorithm=scf_algorithm,
            shift_mode=self._get_shift_mode(shifts),
        )
        self._set_shifts_kpoints(inp, structure, kppa, shifts)
        return inp

    def on_restart(self, abinit_input):
        """Perform updates of AbinitInput upon restart.
        In this case, a static calculation can be started from a relaxation one.
        The relaxation-like variables need to be removed from the AbinitInput.
        """
        # Always remove relaxation-like variables so that if we make the SCF job
        # starting from a previous relaxation or molecular dynamics job, the
        # structure will indeed be static.
        abinit_input.pop_vars(["ionmov", "optcell", "ntime"])

This could be refactored to:

@dataclass
class StaticSetGenerator(AbinitInputGenerator):
    """Common class for ground-state generators."""

    calc_type: str = "static"
    factory: Callable = scf_input
    restart_from_deps: tuple = (f"{SCF}|{RELAX}|{MOLECULAR_DYNAMICS}:WFK|DEN",)
    factory_kwargs : dict = field(
        default_factory=lambda: {
            "accuracy": "normal",
            "spin_mode": "polarized",
            "smearing": "fermi_dirac:0.1 eV",
            "charge": 0.0,
            "shifts": "Monkhorst-Pack",
        }
    )
    user_abinit_settings : dict = field(
        default_factory=lambda: {
            "ionmov": None,
            "optcell": None,
            "ntime": None,
        }
    )

A few points on this:

The None values have been removed if these are just the default settings anyway?
Supplied a factory function.
For settings that you want to remove (e.g., ntime), can you just set them to None?

Similarly, for RelaxSetGenerator it currently looks like this:

@dataclass
class RelaxSetGenerator(GroundStateSetGenerator):
    """Class to generate Abinit relaxation input sets."""

    calc_type: str = "relaxation"
    relax_cell: bool = True
    tolmxf: float = 5.0e-5

    def get_abinit_input(
        self,
        structure=None,
        pseudos=None,
        prev_outputs=None,
        kppa=GroundStateSetGenerator.kppa,
        ecut=GroundStateSetGenerator.ecut,
        pawecutdg=GroundStateSetGenerator.pawecutdg,
        nband=GroundStateSetGenerator.nband,
        accuracy=GroundStateSetGenerator.accuracy,
        spin_mode=GroundStateSetGenerator.spin_mode,
        smearing=GroundStateSetGenerator.smearing,
        charge=GroundStateSetGenerator.charge,
        scf_algorithm=GroundStateSetGenerator.scf_algorithm,
        shifts=GroundStateSetGenerator.shifts,
        relax_cell=relax_cell,
        tolmxf=tolmxf,
        **kwargs,
    ):
        if structure is None:
            raise RuntimeError("Structure is mandatory for RelaxSet generation.")
        if prev_outputs is not None:
            raise RuntimeError(
                "Previous outputs not allowed for RelaxSetGenerator. "
                "To restart from a previous static or otherwise scf "
                "(e.g. relaxation) calculation, use restart_from argument of "
                "get_input_set method instead."
            )
        if kwargs.get("atoms_constraints", None) is not None:
            raise NotImplementedError("Atoms constraints not implemented.")

        ind = 1 if relax_cell else 0
        relax_input = ion_ioncell_relax_input(
            structure,
            pseudos=pseudos,
            kppa=kppa,
            nband=nband,
            ecut=ecut,
            pawecutdg=pawecutdg,
            accuracy=accuracy,
            spin_mode=spin_mode,
            smearing=smearing,
            charge=charge,
            scf_algorithm=scf_algorithm,
            shift_mode=self._get_shift_mode(shifts),
        )[ind]
        relax_input["tolmxf"] = tolmxf
        self._set_shifts_kpoints(relax_input, structure, kppa, shifts)

        return relax_input

Could go to something like:

@dataclass
class RelaxSetGenerator(GroundStateSetGenerator):
    """Class to generate Abinit relaxation input sets."""

    calc_type: str = "relaxation"
    factory: Callable = ion_ioncell_relax_input
    relax_cell: bool = True
    factory_kwargs : dict = field(
        default_factory=lambda: {
            "accuracy": "normal",
            "spin_mode": "polarized",
            "smearing": "fermi_dirac:0.1 eV",
            "charge": 0.0,
            "shifts": "Monkhorst-Pack",
        }
    )
    user_abinit_settings: dict = field(lambda: {"tolmxf": 5.0e-5})
    structure_required: bool = field(True, init=False)
    allow_from_prev: bool = field(False, init=False)

    def get_abinit_input(
        self, structure=None, pseudos=None, prev_outputs=None,
    ):
        input_set = super().get_abinit_input(structure=structure, pseudos=psuedos, prev_outputs=prev_outputs)
        ind = 1 if self.relax_cell else 0
        return input_set[ind]

Some notes on this input set:

The structure and from_prev checks seem like a common thing and are now implemented in the base generator. I wonder if this could actually just be inferred from the restart_from_deps and prev_output_deps?
Used the user_abinit_settings to define settings that get applied to the inputset on creation.
Obviously, in this case you need to do something with the output of the factory method but the get_abinit_input function signature is cleaner and doesn't include all of the factory kwargs. These are all defined in the field itself and applied by the super function.

For the Non scf maker, I'm actually quite unhappy with how I implemented it in atomate2. I actually think it makes sense to split the inputset generators into two, one for uniform and one for line mode since they are actually different settings that you need. Your current code is this:

@dataclass
class NonSCFSetGenerator(AbinitInputGenerator):
    """Class to generate Abinit non-SCF input sets."""

    calc_type: str = "nscf"

    nbands_factor: float = 1.2
    accuracy: str = "normal"
    ndivsm: int = 15
    kppa: float = 1000
    shifts: Union[str, tuple] = "Monkhorst-Pack"

    dos_method: Optional[str] = None
    projection: str = "l"

    mode: str = "line"

    restart_from_deps: tuple = (f"{NSCF}:WFK",)
    prev_outputs_deps: tuple = (f"{SCF}:DEN",)

    def __post_init__(self):
        """Ensure mode is set correctly."""
        self.mode = self.mode.lower()

        supported_modes = ("line", "uniform")
        if self.mode not in supported_modes:
            raise ValueError(f"Supported modes are: {', '.join(supported_modes)}")

    def get_abinit_input(
        self,
        structure=None,
        pseudos=None,
        prev_outputs=None,
        nbands_factor=nbands_factor,
        accuracy=accuracy,
        ndivsm=ndivsm,
        kppa=kppa,
        shifts=shifts,
        dos_method=dos_method,
        projection=projection,
        mode=mode,
    ):
        """Get AbinitInput object for Non-SCF calculation."""
        if prev_outputs is None:
            raise RuntimeError(
                "No previous_outputs. Cannot perform non-SCF calculation."
            )
        if len(prev_outputs) != 1:
            raise RuntimeError(
                "Should have exactly one previous output (an SCF calculation)."
            )
        prev_output = prev_outputs[0]
        previous_abinit_input = load_abinit_input(prev_output)
        previous_structure = get_final_structure(prev_output)
        previous_abinit_input.set_structure(previous_structure)
        if structure is not None:
            if structure != previous_structure:
                raise RuntimeError(
                    "Structure is provided in non-SCF input set generator but "
                    "is not the same as the one from the previous (SCF) input set."
                )
        nband = previous_abinit_input.get(
            "nband",
            previous_abinit_input.structure.num_valence_electrons(
                previous_abinit_input.pseudos
            ),
        )
        nband = int(np.ceil(nband * nbands_factor))
        if mode == "line":
            return ebands_from_gsinput(
                gs_input=previous_abinit_input,
                nband=nband,
                ndivsm=ndivsm,
                accuracy=accuracy,
            )
        elif mode == "uniform":
            if dos_method:
                uniform_input = dos_from_gsinput(
                    gs_input=previous_abinit_input,
                    kppa=kppa,
                    nband=nband,
                    accuracy=accuracy,
                    dos_method=dos_method,
                    projection=projection,
                    shift_mode=self._get_shift_mode(shifts),
                )
            else:
                uniform_input = nscf_from_gsinput(
                    gs_input=previous_abinit_input,
                    kppa=kppa,
                    nband=nband,
                    accuracy=accuracy,
                    shift_mode=self._get_shift_mode(shifts),
                )
            self._set_shifts_kpoints(uniform_input, structure, kppa, shifts)
            return uniform_input
        else:
            raise RuntimeError(
                f"'{self.mode}' is wrong mode for {self.__class__.__name__}."
            )

I wonder if it could look like:

@dataclass
class LineNonSCFSetGenerator(AbinitInputGenerator):
    """Class to generate Abinit non-SCF input sets."""

    calc_type: str = "nscf"
    factory: Callable = ebands_from_gsinput
    restart_from_deps: tuple = (f"{NSCF}:WFK",)
    prev_outputs_deps: tuple = (f"{SCF}:DEN",)
    nbands_factor: float = 1.2
    factory_kwargs : dict = field(
        default_factory=lambda: {
            "accuracy": "normal",
            "ndivsm": 15,
            "kppa": 1000,
            "shifts": "Monkhorst-Pack",
        }
    )

    def get_abinit_input(
        self,
        structure=None,
        pseudos=None,
        prev_outputs=None,
    ):
        """Get AbinitInput object for Non-SCF calculation."""
        self.factor_kwargs["nband"] = _get_nband(prev_outputs, nbands_factor=self.nbands_factor)
        return super().get_abinit_input(structure=structure, psuedos=psuedos, prev_outputs=prev_outputs)
        
def _get_nband(prev_outputs, nbands_factor=1.):
    prev_output = prev_outputs[0]
    previous_abinit_input = load_abinit_input(prev_output)
    nband = previous_abinit_input.get(
        "nband",
        previous_abinit_input.structure.num_valence_electrons(
            previous_abinit_input.pseudos
        ),
    )
    return int(np.ceil(nband * nbands_factor))

In some sense, part of the difficulty is that the input set factory methods in abipy are a little bumpy in that their inputs/outputs are quite variable. So I guess a lot of what we have to do in atomate2 is smooth over those bumps. The issue is that because of the factories, if a user wants to customise the factory kwargs, they will have to go to abipy to see what settings are available for that factory method. And by wrapping it in the atomate2 generator, we almost have a nested factory which is complicated. One thing would be to effectively rewrite the abipy factories directly in atomate2 but this would be a lot of work and so probably not recommended.

Also, I realise it is much easier for me to write pseudo code without having to deal with the practicalities of the code, so I realise it might not be possible to get it this clean. But I think some standardisation will definitely help with the usability.

utf · 2023-03-21T21:52:43Z

The other things which I'm not sure about are the BaseAbinitMaker.from_params and BaseAbinitMaker.from_prev_maker functions. Are these actually needed? It would be nice to just be able to link jobs just by using prev_output or restart_from. These other functions add quite a bit of complexity and many more options for creating makers/jobs which could be confusing for users.

Abinit workflows

utf

Hi @davidwaroquiers @gpetretto, this is looking excellent! Mostly only cosmetic changes.

One big remaining thing is the task document. Currently no outputs are stored which is a big limitation. What do you think about following the template of the VASP schemas? The CP2K schema is almost exactly the same as VASP so I think if you stay along those lines we should be fine to abstract it out at a later point.

utf · 2023-05-11T16:12:14Z

src/atomate2/abinit/jobs/base.py

+    run_abinit_kwargs: dict = field(default_factory=dict)
+
+    # class variables
+    CRITICAL_EVENTS: ClassVar[Sequence[str]] = ()


I also think it makes sense to the objects themselves.

utf · 2023-05-11T16:12:53Z

src/atomate2/abinit/jobs/base.py

+        prev_outputs: str | Path | list[str] | None = None,
+        restart_from: str | Path | list[str] | None = None,
+        history: JobHistory | None = None,
+    ) -> jobflow.Flow | jobflow.Job:


Will this ever return a Flow?

utf · 2023-05-11T16:14:25Z

src/atomate2/abinit/jobs/base.py

+            return Response(
+                output=task_document,
+                stop_children=True,
+                stop_jobflow=True,


Why stop Jobflow? I think this should just be stop children as there may be other parts of the workflow that you want to keep running.

utf · 2023-05-11T16:15:11Z

src/atomate2/abinit/jobs/base.py

+                output=task_document,
+                stop_children=True,
+                stop_jobflow=True,
+                stored_data={"error": unconverged_error},


Any updated thoughts on this?

utf · 2023-05-11T16:17:28Z

src/atomate2/abinit/jobs/core.py

+        # TODO: add the possibility to tune the RelaxInputGenerator options
+        #  in this class method.
+        return cls(
+            # input_set_generator=RelaxSetGenerator(relax_cell=True, *args, **kwargs)


This line seems like it should provide full control. Any reason it is commented out?

utf · 2023-05-11T16:23:38Z

src/atomate2/abinit/schemas/core.py

+    """Definition of task document about an Abinit Job."""
+
+    state: Status = Field(None, description="State of this job.")
+    run_number: int = Field(None, description="Run number of this job.")


Is this still needed? Is this not contained in the history? On that note, do you want to store the history in the task document.

utf · 2023-05-11T16:25:20Z

src/atomate2/abinit/sets/base.py

+    restart_from_deps:
+        Defines the files that needs to be linked from previous calculations in
+        case of restart. The format is a tuple where each element is a list of
+        "|" separated runelevels (as defined in the AbinitInput object) followed


Suggested change

"|" separated runelevels (as defined in the AbinitInput object) followed

"|" separated run levels (as defined in the AbinitInput object) followed

utf · 2023-05-11T16:25:41Z

src/atomate2/abinit/sets/base.py

+        Defines the files that needs to be linked from previous calculations and
+        are required for the execution of the current calculation.
+        The format is a tuple where each element is a list of  "|" separated
+        runelevels (as defined in the AbinitInput object) followed by a colon and


Suggested change

runelevels (as defined in the AbinitInput object) followed by a colon and

run levels (as defined in the AbinitInput object) followed by a colon and

utf · 2023-05-11T16:31:35Z

src/atomate2/abinit/sets/base.py

+        input_index
+            The index to be used to select the AbinitInput in case a factory
+            returns a MultiDataset.
+


Remove extra empty space.

utf · 2023-05-11T16:33:12Z

src/atomate2/abinit/sets/core.py

+    calc_type: str = "static"
+    factory: Callable = scf_input
+    restart_from_deps: tuple = field(
+        default_factory=lambda: tuple(GS_RESTART_FROM_DEPS)


No need for the default_factory stuff here and in other generators where you define restart_from_deps. That's because tuples are immutable anyway, so you can just definitely this as:

restart_from_deps: tuple = (GS_RESTART_FROM_DEPS, )

Zhuoying · 2023-10-12T04:25:35Z

Hi @davidwaroquiers @gpetretto a quick catch-up with this PR:
It seems @utf has reviewed it very carefully. Only a few cosmetic changes and the following errors in tests need to be done before we merge this PR.
The schemas of Abinit output can be discussed in Issues or submitted in another PR since this PR was already quite large.

…it_workflows

…it_workflows pull trunk main

Abinit workflows

davidwaroquiers · 2024-03-12T18:42:57Z

@utf This should be ready to be reviewed (and hopefully merged). @VicTrqt and @gpetretto did a great work of tackling the different issues, comments and suggestions. It took also some more time due to versions compatibility issues between pymatgen abipy and other packages. The workflow needs approval for the testing.

utf · 2024-03-28T10:39:07Z

Hi @davidwaroquiers, thanks very much for this. I've looked over and it looks ready to merge.

The last thing I want to ask is about moving the inputsets to pymatgen. Do you see any barrier to this?

davidwaroquiers · 2024-03-28T10:48:17Z

Hi @davidwaroquiers, thanks very much for this. I've looked over and it looks ready to merge.

The last thing I want to ask is about moving the inputsets to pymatgen. Do you see any barrier to this?

Hi @utf ,

Great, thanks a lot for checking this huge PR :)

Actually for the input sets, there is indeed an issue in doing that. The input sets use abipy's functionalities and abipy depend on pymatgen. So if we move the input sets to pymatgen, there will be a circular dependency problem. We already thought about this with @gpetretto and one option could be to move it to abipy, which could somehow make sense. Another possibility could be to make an add-on to pymatgen for the abinit input sets ? Not sure if this 1/ would work for the circular dependency and 2/ would actually really make sense.

What do you think ?

utf · 2024-03-28T10:50:50Z

Thanks for the quick response. Personally, I think moving them to abipy could make sense? That way other abipy users could make use of the input sets too.

davidwaroquiers · 2024-03-29T14:27:41Z

Thanks for the quick response. Personally, I think moving them to abipy could make sense? That way other abipy users could make use of the input sets too.

Yup we will go that way. Hopefully we have this done by next week (and a new abipy release is also made)

gpetretto · 2024-04-08T11:52:37Z

Thanks for the quick response. Personally, I think moving them to abipy could make sense? That way other abipy users could make use of the input sets too.

Hi @utf, I have implemented the required changes to move the input sets to abipy. However, since we hope to add a few more workflow before the atomate2 paper is finalised, this means that we would need to rely on making new releases of abipy each time we add a new workflow. Since the time for the paper is relatively strict, we thought that it may be worth merging this as it is and move the input sets after we have finalised more workflows. What do you think?

utf · 2024-04-08T11:53:47Z

Sounds good to me! Thanks everyone involved in this PR. Excited to have it in atomate2!

utf · 2024-04-08T11:55:24Z

I can't merge at the moment:

This branch has conflicts that must be resolved
Use the command line to resolve conflicts before continuing.

JaGeo · 2024-04-08T12:01:39Z

(@gpetretto is there a chance for another (harmonic) phonon workflow? :D)

gpetretto · 2024-04-08T12:43:40Z

I can't merge at the moment:

This branch has conflicts that must be resolved
Use the command line to resolve conflicts before continuing.

I resolved the conflicts. You should be able to merge as soon as David merges my PR to his branch davidwaroquiers#93

(@gpetretto is there a chance for another (harmonic) phonon workflow? :D)

We will try to make a phonopy workflow based on the common one. Probably the DFPT will take longer.

JaGeo · 2024-04-08T12:45:21Z

@gpetretto That would be awesome.

Resolve conflicts

davidwaroquiers · 2024-04-08T13:25:08Z

Hi @utf

I just merged Guido's resolution of conflicts, this PR just needs approval for the tests to run now and hopefully you are able to merge afterwards.

Thanks a lot!

* Abinit workflows: static, non-scf and relaxation. * Various changes based on GP's comments. * Removed usage of pmg_serialize. * Fixed issues with changes in current atomate2. * Fixed linting. * More linting. * More modifications + refactored tests to functions only. * change logic of input set generator. update tests * update abipy version * fix lint * more lint * PR review start * linting * linting+pytest * Start schemas like Aims/CP2K * Start schemas like Aims/CP2K * abinit schemas * Optional * test print files * remove schemas/core from import * outdata files * remove if TYPE_CHECKING Vector3D * gsr abinit version * abinit_objects to None for the time being * trajectory * debugging * functioning abinit workflows schemas * debug * switch critical_events into objects instead of str * fix how to check for unconverged states * remove Molecule * remove unused PrevOutput + use atomate2.utils.datetime + factory Callable jobflow fix * linting * remove critical_events from calculation * remove commented code * linting + pass tests * linting --------- Co-authored-by: Guido Petretto <[email protected]> Co-authored-by: Victor Trinquet <[email protected]>

Abinit workflows: static, non-scf and relaxation.

3aa383b

gpetretto reviewed Nov 2, 2022

View reviewed changes

davidwaroquiers and others added 14 commits March 22, 2023 12:12

Various changes based on GP's comments.

4bb9fa1

Merge remote-tracking branch 'upstream/main' into abinit_workflows

1dd5498

Removed usage of pmg_serialize.

1310afb

Fixed issues with changes in current atomate2.

a5e0998

Fixed linting.

e90797f

More linting.

94c0021

More modifications + refactored tests to functions only.

cd536cb

change logic of input set generator. update tests

74a2158

update abipy version

458377a

Merge remote-tracking branch 'upstream/main' into abinit2

46f6470

fix lint

ebe4411

more lint

676432a

Merge pull request #90 from gpetretto/abinit2

ed25422

Abinit workflows

Merge branch 'main' into abinit_workflows

01cb553

davidwaroquiers requested a review from utf May 9, 2023 08:49

utf reviewed May 11, 2023

View reviewed changes

davidwaroquiers added 2 commits September 4, 2023 09:23

Merge remote-tracking branch 'upstream/main' into abinit_workflows

9164ab4

Merge remote-tracking branch 'upstream/main' into abinit_workflows

8522bde

janosh force-pushed the main branch from 6ceb1ec to 3764841 Compare October 27, 2023 00:12

VicTrqt added 2 commits November 8, 2023 16:27

Merge branch 'main' of github.com:materialsproject/atomate2 into abin…

d29c6f2

…it_workflows

PR review start

b35d654

VicTrqt and others added 8 commits February 15, 2024 10:58

remove commented code

37a617a

linting

689a616

linting + pass tests

e840f9b

pull trunk main

2bbfba4

Merge branch 'main' of github.com:materialsproject/atomate2 into abin…

f61f72d

…it_workflows pull trunk main

pull trunk main

ed197c3

linting

293cabd

Merge pull request #92 from VicTrqt/abinit_workflows

66dc14a

Abinit workflows

utf enabled auto-merge (squash) April 8, 2024 11:53

utf disabled auto-merge April 8, 2024 11:54

utf enabled auto-merge (squash) April 8, 2024 11:54

Merge remote-tracking branch 'upstream/main' into abinit_merge

e787998

Merge pull request #93 from gpetretto/abinit_merge

a27caaf

Resolve conflicts

auto-merge was automatically disabled April 8, 2024 13:23
Head branch was pushed to by a user without write access

utf enabled auto-merge (squash) April 8, 2024 13:25

utf merged commit f26b43b into materialsproject:main Apr 8, 2024
6 checks passed

utf added the feature A new feature being added label Aug 20, 2024

		energy: float = Field(None, description="Final energy of the calculation.")


		class AbinitTaskDocument(StructureMetadata):

	"\|" separated runelevels (as defined in the AbinitInput object) followed
	"\|" separated run levels (as defined in the AbinitInput object) followed

	runelevels (as defined in the AbinitInput object) followed by a colon and
	run levels (as defined in the AbinitInput object) followed by a colon and

Abinit workflows: static, non-scf and relaxation. #183

Abinit workflows: static, non-scf and relaxation. #183

Conversation

davidwaroquiers commented Oct 18, 2022

Summary

Additional dependencies introduced (if any)

TODO

davidwaroquiers commented Oct 18, 2022

codecov bot commented Oct 18, 2022 • edited Loading

Codecov Report

gpetretto left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

utf commented Mar 21, 2023

davidwaroquiers commented Mar 21, 2023

utf commented Mar 21, 2023 • edited Loading

utf commented Mar 21, 2023

utf left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Zhuoying commented Oct 12, 2023

davidwaroquiers commented Mar 12, 2024

utf commented Mar 28, 2024

davidwaroquiers commented Mar 28, 2024

utf commented Mar 28, 2024

davidwaroquiers commented Mar 29, 2024

gpetretto commented Apr 8, 2024

utf commented Apr 8, 2024

utf commented Apr 8, 2024

JaGeo commented Apr 8, 2024

gpetretto commented Apr 8, 2024

JaGeo commented Apr 8, 2024

davidwaroquiers commented Apr 8, 2024

codecov bot commented Oct 18, 2022 •

edited

Loading

utf commented Mar 21, 2023 •

edited

Loading