aiidateam / aiida-quantumespresso

The official AiiDA plugin for Quantum ESPRESSO
https://aiida-quantumespresso.readthedocs.io
Other
52 stars 77 forks source link

Periodicity #896

Closed AndresOrtegaGuerrero closed 1 year ago

AndresOrtegaGuerrero commented 1 year ago

This PR implements a class type PeriodicityType so the user can set the periodicity of kpoints grid in the QE simulations (For base and relax worchain)

There are implemented 3 types of periodicity , 'x' , 'xy', and 'xyz' The intention of this class is allow the workchains to adapt when systems are 1D, or 2D

Example 2D System (Slab) the KPOINTS grid we are interested are n x n x 1 For relax simulations , the periodicity is taken into account to apply the corresponding cell_dofree contraints

AndresOrtegaGuerrero commented 1 year ago

Hello @mbercx I was wondering if you could help me figuring out what could be the problem with theh doc/readthedocs ?

mbercx commented 1 year ago

Thanks @AndresOrtegaGuerrero! I think the docs are just failing because of a RTD build hiccup. Just rerunning the build might work fine.

More importantly though, I'm wondering if the best solution for dealing with <3D structures is to use an extra periodicity input. The StructureData class already has a pbc attribute that indicates the periodic boundary conditions (i.e. the dimensionality):

In [1]: structure = load_node(14808)

In [2]: structure.pbc
Out[2]: (True, True, True)

I'm wondering if we shouldn't simply rely on this attribute to adapt the inputs of the various work chains?

AndresOrtegaGuerrero commented 1 year ago

@mbercx Thank you for your response , I could reformulate the PR so it takes into account the periodicity set in the structure data. However one issue I might encounter is that once the StructureData is stored , then it cant be modified. Thinking about the QE App I would still reformulate create_kpoints_from_distance , where the StructureData is the input so it can return the kpoints accordingly like 1D , 2D or 3D

mbercx commented 1 year ago

Before you do, I think @demiranda-gabriel has already done some work on this, and may have some code/insights to offer.

However one issue I might encounter is that once the StructureData is stored , then it cant be modified. Thinking about the QE App

True, you'd have to create a new StructureData with the proper periodic boundary conditions. Still, I think this is a cleaner approach compared to adding a periodicity input everywhere, since the information is technically already contained in the StructureData class.

Also: Typically a cell that is used to run a <3D calculation should have a bunch of vacuum in it, no? So having pbc == [True, True, True] for such a StructureData would be erroneous?

demiranda-gabriel commented 1 year ago

Hey! Thanks for including me in the discussion. Indeed, @AndresOrtegaGuerrero and I plan to work together on similar things in the very near future :)

In fact, create_kpoints_from_distance should already provide the expected behavior for structures with low dimensionality (that is, that lack any pbc direction). For instance, it would indeed create a k-mesh of the type n x n x 1 if structure.pbc == (True, True, False), based on the implementation of the set_kpoints_mesh_from_density subfunction here. So the pbc attribute should already take care of this part.

Now, @mbercx's last point is an important question as well. Usually one would have to make changes to the structure itself depending on if one would like to run a calculation on the 2D or 3D framework. For example, one usually has to ensure that the unit cell has a proper vacuum distance along the non-periodic direction in a 2D framework calculation, besides the other changes in the calculation flags (e.g. changing the cell_dofree relaxation constraints and the assume_isolated flag to '2D' for proper isolation of the monolayer from its periodic images). While a reasonable vacuum distance along the stacking direction should be enough in the 2D framework, one would usually want to naturally change it if doing anything on the 3D framework (e.g. to calculate the interlayer distance in the bulk form or any other bulk-related property).

However, I can imagine that Andreas was looking for a flexible way for the user to change the 1D/2D/3D framework of the calculation within the QE App. What I have been thinking about for some time now and would suggest here is the implementation of some kind of structure_preparation calcfunction that the app can run before submitting an actual calculation. This step could be then responsible for creating a new structure with the proper pbc chosen by the user, and other adjustments that could be desired (e.g. changing the vacuum distance for the 2D case) in a flexible way, before using it as input for the actual calculations.

I believe that this would bypass the need to inject the dependency of this new PeriodicityType into the code, as everything could rely on the structure.pbc itself. However, a modification on the base and relax workchains for instance are still needed to adapt the cell_dofree, assume_isolated and other relevant flags based on the structure periodic boundary conditions.

sphuber commented 1 year ago

Thanks for the PR and the discussion guys.

I believe that this would bypass the need to inject the dependency of this new PeriodicityType into the code, as everything could rely on the structure.pbc itself. However, a modification on the base and relax workchains for instance are still needed to adapt the cell_dofree, assume_isolated and other relevant flags based on the structure periodic boundary conditions.

I personally would also go in this direction. Have the code "respect" the pbc attribute of the structure and adapt parameters accordingly in the various workchains. And indeed, we then just need some basic calcfunctions to take a StructureData and add vacuum where necessary and set the pbc accordingly`. I think this should satisfy all requirements and use-cases without adding new concepts and duplicating data.

AndresOrtegaGuerrero commented 1 year ago

Hello, thank you for all your comments, so given the need for this feature in the QE app, I could in principle modify the PR , to just read the pbc of the StructureData file. I thought of the flexibility that a Periodicity class could bring (Like Relax and Spin and Electronic types), since you could have different directions and options. In the case of the QE App, we have editors where the user can modify the vacuum length and supercell size before we set the attributes for the workchain. I guess we can move forward with an implementation based on Structure.pbc within the qe-pluggin. In the QE App, we could add an editor to modify the StructureData so the user selects the periodicity, before is saved. What do you think @unkcpz ?

unkcpz commented 1 year ago

Thanks all for the discussion!

In the case of the QE App, we have editors where the user can modify the vacuum length and supercell size before we set the attributes for the workchain. I guess we can move forward with an implementation based on Structure.pbc within the qe-pluggin.

I agree with this, we already using the same concept in QeApp to only use the modified structure as the input to QeAppWorkChain. By this approach, I think it decouples the structure setting and workchain running clearly, which gives maximum flexibility to the user as well.

AndresOrtegaGuerrero commented 1 year ago

I decided to do a new PR since it was easier #907 @mbercx @sphuber @demiranda-gabriel