SCM-NV / pyZacros

Python Library for Automating Zacros Simulations
Other
7 stars 2 forks source link

Instantiate ZacrosJob #78

Closed lopeztarifa closed 2 years ago

lopeztarifa commented 2 years ago

Hi @nfaguirrec

For the ZacrosJob class, would it be possible to maintain the original init of the SingleJob? That is to be able to instantiate the object without any argument, like the AMSJob class:

a=AMSJob()

a=ZacrosJob()
TypeError: __init__() missing 3 required positional arguments: 'lattice', 'mechanism', and 'cluster_expansion'

I would like to instantiate the object and then stuff the lattice, mechanism, and cluster_expansion terms. Something in these lines:

 if isinstance(a, AMSjob):
     a.molecule=plams_molecule
     a.settings=plams_settings
 elif isinstance(a, ZacrosJob):
     a.lattice=pz_lattice
     a.mechanism=pz_mechanism
     a.cluster_expansion=pz_cluster_expansion

Thanks

nfaguirrec commented 2 years ago

Hi @lopeztarifa,

It is possible, but modifying the code considerably, and I think it is not a good idea to do so. In the constructor, there are some checks of the consistency among the lattice, mechanism, and cluster expansion, and also the determination of other properties like the molar fractions. The same applies to AMSJob. Its constructor also checks the Settings type object before storing it.

Without knowing more details about your code, it would do something like this:

if isinstance(a, AMSjob):
     a = AMSJob(plams_molecule, plams_settings)
 elif isinstance(a, ZacrosJob):
     a = ZacrosJob(pz_lattice, pz_mechanism, pz_cluster_expansion)
lopeztarifa commented 2 years ago

Hi @nfaguirrec,

Thanks for your reply, here is more info, I hope it helps :) The elif solution does not work as the isinstance function only acts on instantiated objects, e.g.

a=AMSJob()
print(isinstance(a, AMSjob))
>> True

a=ZacrosJob()
print(isinstance(a, ZacrosJob))
>> TypeError: __init__() missing 3 required positional arguments: 'lattice', 'mechanism', and 'cluster_expansion'

More context:

I am dealing with the problem of mapping between semantic/syntactic workflows.

For each engine I need to develop a wrapper session, that starts by instantiating the syntactic main object, eg. AMSJob for the AMS wrapper:

from scm.plams import AMSJob

class SimamsSession(SimWrapperSession):
    """Describe AMS Session class."""

    def __init__(self, engine=None, **kwargs):
        """Initialise SimamsSession."""
        if engine is None:
            PlamsInit()
            self.engine = AMSJob()   #  or eg. = PyLammps() for LAMMPS
        super().__init__(engine, **kwargs)

I would like to develop a similar wrapper but with

self.engine = ZacrosJob()  

Then there is a function

map_function(root_obj, self.engine)

that carries out the mapping in the CUDS object adding the arguments according to the 'engine' class type:

map_function(root_obj, self.engine)

def map_function(root_cuds_object: Cuds, engine)-> tuple:

    if isinstance(engine, AMSJob):             # Possible
        plams_molecule = map_PLAMSMolecule(root_cuds_object) 
        plams_settings = map_PLAMSSettings(root_cuds_object)
       engine(molecule=plams_molecule, settings=plams_settings)
    elif isinstance(engine, ZacrosJob):       # not possible atm
        pz_lattice = map_Lattice(root_cuds_object) 
        pz_mechanism = map_Mechanism(root_cuds_object)
        pz_cluster_expansion = map_ClusterExpansion(root_cuds_object)
       engine(lattice=pz_lattice, mechanism=pz_mechanism, cluster_expansion=pz_cluster_expansion)

Question

Like the AMSJob() or PyLammps(), could the ZacrosJob() be instantiated without any argument?

In my opinion, it will give the flexibility to store the lattice and the mechanism and use the a.mechanism and a.lattice to trigger AMS calculations for calculating the cluster_expansions for instance. Just wild thinking. The checks on the arguments can always happen before executing the run method.

If not possible I will think of a workaround. Thanks!

lopeztarifa commented 2 years ago

After a discussion, the change will mess up the code too much. Is not worth it.

I close the issue.