aiida-vasp / parsevasp

A general parser for VASP
MIT License
13 stars 13 forks source link

How to convert a poscar instance to an aiida StrutcureData ? #117

Open sylviancadars opened 2 years ago

sylviancadars commented 2 years ago

Hi, I cannot find a way to perform as simple a task as converting my parsevasp.poscar.Poscar instance to an aiida.orm.StrutcureData object. Isn't there a simple method to do this ?

I found : aiida_vasp.parsers.content_parsers.poscar import parsevasp_to_aiida

But I fail to see how to use it as the doc isn't really helpful (just says that it returns "a dictionary representation which are ready to be used when creating an AiiDA StructureData instance.)

What I ultimately want to do was to retrieve the CONTCAR from the remote directory of an unfinished aiida-vasp calculation and use it to update my self.ctx.inputs.structure in a workchain.

I know I can do it from pymatgen or ASE, but it seemed to make more sense to use the parsevasp package to do this. Am I wrong ?

Can anyone please help ? Thanks. All the best. Sylvian.

zhubonan commented 2 years ago

parsevasp is made without any dependencies on aiida on purpose so it can be used/maintained independently. If you want to use parsevasp.poscar.Poscar for StructureData you adapt the following code:

https://github.com/aiida-vasp/aiida-vasp/blob/59c816be3844d9bada6c7e4cb615bed8d06a6047/aiida_vasp/parsers/node_composer.py#L130-L149

What I ultimately want to do was to retrieve the CONTCAR from the remote directory of an unfinished aiida-vasp calculation and use it to update my self.ctx.inputs.structure in a workchain.

This can be done by passing builder.settings = Dict(dict={'parser_settings': 'add_poscar-structure'}) which will trigger outputing the parsed structure from the CONTCAR (despite the name poscar-structure). Then the parsed StructureData can be assesed with node.outputs.structure. Also you don't need to retrieve CONTCAR manually in the workflow as it already retrieved for VaspCalculation.

However, the parsed StructureData does not contain the velocities so that is something need to be added. I can see that the Poscar parser does support parsing/writing the velocity.

So we will need to modify aiida-vasp's parser to include the velocities in some way.

  1. One options is to include the velocities as extra attribute to the StructureData, then the same StructureData can be used to write the next POSCAR with this information included.

  2. The alternative is to have an extra array output containing the velocities can make VaspCalculation accept an extra input port.

I think the first solution is neater although we deviate a bit from the standard StructureData specification. But I don't think it is something forbidden. Option two will involve more changes to the parser and VaspCalculation. @espenfl What do you think?

sylviancadars commented 2 years ago

Thanks @zhubonan for your answer,

Yes of course, I forgot to mention in my message the velocities and I am glad you pointed this out. Especially since it might trigger new developpements. (From a very personal point a view I am a little bit concerned however future developements may not be appicable to my particular case because, as I mentioned in relation to some other issues a few weeks ago, I still have to use aiida-1.6.8 for the moment for compatibility issues.)

Now concerning the possibility to pass builder.settings = Dict(dict={'parser_settings': 'add_poscar-structure'}), which I hadn't noticed I would not have gessed anyways used the CONTCAR file, does it output node.outputs.structure even though the calculation is not finished ? I'll try this out.

It took me some time to find out how to get a structure out the outputs.retrieved node but I finally managed to do it with:

self.ctx.inputs.structure = aiida.orm.StructureData(pymatgen=pymatgen.core.structure.Structure.from_str( node.outputs.retrieved.get_object_content('CONTCAR'), fmt='poscar'))

And finally, another part concerns the possibility to build a trajectory out of such unfinished calculations. Is parsevasp.vasprun.Xml() the way to go go (in which case the same question of the conversion to an aiida object - TrajectoryData in this case -) or should I use some aiida_vasp functionnality instead ? I am afraid I will also need help for that part and can create another issue therefore if needed.

Thank you (again and again) for your help. All the best. Sylvian.

sylviancadars commented 2 years ago

I just got a result from a test of your suggestion, which I implemented as follows :

settings = AttributeDict()
settings.parser_settings = {'output_params': ['total_energies',
                                                  'maximum_force'],
                                'add_trajectory': True,
                                'add_forces': True,
                                'add_energies': True, 
                                'add_poscar-structure': True}
inputs.settings = Dict(dict=settings)

verdi process show: Property Value


type VaspCalculation state Finished [1004] the parser is not able to compose one or more output nodes: poscar-structure ...

$ verdi process report 728767 728767: None (empty scheduler output file) *** Scheduler errors: Loading vasp/5.4.4-mpi-cuda Loading requirement: intel-compilers/19.0.4 intel-mpi/2019.4

*** 3 LOG MESSAGES: +-> WARNING at 2022-08-22 16:25:35.124571+00:00 | poscar-structure has been requested, however its parser has not been implemented. Please check the docstrings in aiida_vasp.parsers.vasp.py for valid input. +-> WARNING at 2022-08-22 16:25:35.206590+00:00 | Creating node structure of type structure failed. No parsed data available. +-> WARNING at 2022-08-22 16:25:35.209656+00:00 | output parser returned exit code<1004>: the parser is not able to compose one or more output nodes: poscar-structure

Best. Sylvian

sylviancadars commented 2 years ago

I will "check the docstrings in aiida_vasp.parsers.vasp.py for valid input" as suggested in the report, but not before a couple of days.

espenfl commented 2 years ago

Thanks for opening the issue and @zhubonan for the detailed answers.

  • One options is to include the velocities as extra attribute to the StructureData, then the same StructureData can be used to write the next POSCAR with this information included.

I do not think we should mix in other things than structure in the StructureData. This we have discussed before, related to e.g. selective dynamics, which is more related to the calculation. Same is the velocities in my view. The snapshot is the structure as it is, but the trajectory in and out of that StructureData is really dependent on what happens before and after, which is related to the calculation. So I suggest that we use another data structure or continue what we did for the selective dynamics.

  • The alternative is to have an extra array output containing the velocities can make VaspCalculation accept an extra input port.

Yes, I think this is maybe better. Then we do not clutter the StructureData and it makes it cleaner for the user.

espenfl commented 2 years ago

| poscar-structure has been requested, however its parser has not been implemented. Please check the docstrings in aiida_vasp.parsers.vasp.py for valid input.

Yes, this needs to be fixed. Ultimately I think we should add the restart using velocities functionality sooner than later. If someone wants to give it a go, that would be greatly appreciated.