stfc / aiida-mlip

machine learning interatomic potentials aiida plugin
https://stfc.github.io/aiida-mlip/
BSD 3-Clause "New" or "Revised" License
4 stars 5 forks source link

config file to aiida #108

Open federicazanca opened 7 months ago

federicazanca commented 7 months ago

I had something like this added to the base calcjob class (which I removed for now) The idea would be to make aiida read config files for running, but at the moment we don't want to implement it because it could be something to discuss to add on a aiida-core level (or to check if it is an option already)

@classmethod
    def run_from_config(
        cls, config_file: Union[Path, str], launch: str
    ) -> tuple[dict[str, Any], ProcessNode]:
        """
        Parse config file.

        Parameters
        ----------
        config_file : Filepath
            The config file path.
        launch : str
            Chosen launch function (can be run, run_get_node, run_get_pk, submit).

        Returns
        -------
        tuple
            Returns the output of the chosen launch function.
        """
        with open(config_file, encoding="utf-8") as f:
            config_dict = yaml.safe_load(f)
            config = convert_to_nodes(config_dict, convert_all=True)
        if launch == "run":
            return run(cls, config)
        if launch == "run_get_node":
            return run_get_node(cls, config)
        if launch == "submit":
            return submit(cls, config)
        if launch == "run_get_pk":
            return run_get_pk(cls, config)
        raise ValueError
ElliottKasoar commented 7 months ago

but at the moment we don't want to implement it because it could be something to discuss to add on a aiida-core level (or to check if it is an option already)

I'd imagine adding things to aiida-core could be a relatively slow process? They release quite frequently but with time for discussion, implementation etc, I'd imagine we'd still be looking at months?

If it's fairly useful, would it not make more sense to implement it here relatively soon, then we can swap it out for the aiida-core implementation if it gets that far?

federicazanca commented 7 months ago

but at the moment we don't want to implement it because it could be something to discuss to add on a aiida-core level (or to check if it is an option already)

I'd imagine adding things to aiida-core could be a relatively slow process? They release quite frequently but with time for discussion, implementation etc, I'd imagine we'd still be looking at months?

If it's fairly useful, would it not make more sense to implement it here relatively soon, then we can swap it out for the aiida-core implementation if it gets that far?

Yes if it's useful we could add it in aiida-mlip directly. I am still deciding if it's useful or not, cause at the moment I only seem to have created confusion. For now I put it here since it can be handled as a separate issue from #109, as they don't really depend on each other. If we decide to add this to aiida_mlip we can always do it later, tho

federicazanca commented 7 months ago

basicallhy the aiida implementation for this would be the get_builder fuction so these 2 issues are more or less the same #106. Not sure if we want to deal with this at the moment, possibly in the future