Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

📚 Docs: Add first version of PwRelaxWorkChain how to #934

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

mbercx
Copy link
Member

@mbercx mbercx commented May 20, 2023

First version of the How-to section on the PwRelaxWorkChain. Describes basic usage via the get_builder_from_protocol() method, as well as determining the basic degrees of freedom using the RelaxType enum.

Copy link
Contributor

@sphuber sphuber left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @mbercx . Think this is great to have. I just have two questions now that we restarted the docs from scratch and will start adding pieces by different authors.

I feel the tone of this addition is more that of a tutorial, rather than a how-to guide. Would it make sense to add this rather to the tutorials section? As you can see, the entry I added for PwCalculation is in similar spirit to what you wrote here, in the sense that it is a complete and step-by-step instruction. The changing of relax type would be a great section for the how-to guide though.

Think we should also make some agreements up front on the language. Especially for how-to's, I think we shouldn't use the second person singular, but a more neutral language. If you prefer, for tutorial content, I think it could make sense the way you wrote it. This is always going to be heavily influenced by personal preference, but I think that if we want to have the docs be readable, they should be as consistent as possible, despite being written by different authors.

Comment on lines +17 to +21
```python
from aiida_quantumespresso.workflows.pw.relax import PwRelaxWorkChain

code = orm.load_code('qe-7.2-pw@localhost')
structure = orm.load_node(1) # FCC Silicon
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not make it actually runnable?

Suggested change
```python
from aiida_quantumespresso.workflows.pw.relax import PwRelaxWorkChain
code = orm.load_code('qe-7.2-pw@localhost')
structure = orm.load_node(1) # FCC Silicon
```python
from ase.build import bulk
from aiida_quantumespresso.workflows.pw.relax import PwRelaxWorkChain
code = orm.load_code('qe-7.2-pw@localhost')
structure = orm.StructureData(ase=bulk('Si', 'diamond', 5.43)) # FCC Silicon

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Although having the structure is a bonus, in the end they probably still need to adapt the code ID. ^^

@mbercx
Copy link
Member Author

mbercx commented May 23, 2023

Thanks for the comments, @sphuber! This is exactly why I still left the PR as a WIP, to discuss how to properly structure the docs. 👍 📚

I completely agree that it is good to have a clear separation between tutorials and how-to's, although there will always be a bit of overlap. Also fine with rethinking the language. Let me have a crack at adapting this documentation, but also write down our policies on this so we can crystallise our approach.

@ltalirz
Copy link
Member

ltalirz commented May 30, 2023

Thanks for this @mbercx !

I personally don't feel strongly about the language you choose here, but I do agree about the level of verbosity - I believe less is more here, both for users and for maintainers. As a first step, I would only add a tiny bit of extra information per worfklow - the minimum a new user needs to know which workflow to run, and any specific settings that cannot be left at their default values.

Here are my suggestions

  • Start the top-level howto section with anything that is general to all aiida-qe workflows (e.g. the get_builder_from_protocol pattern)
  • For the PwRelaxWorkChain something like this would suffice:

``python
from aiida_quantumespresso.workflows.pw.relax import PwRelaxWorkChain
from aiida_quantumespresso.common.types import RelaxType

builder = PwRelaxWorkChain.get_builder_from_protocol(
    code=orm.load_code('qe-7.2-pw@localhost'),
    structure=my_structure,  # StructureData
    relax_type=RelaxType.CELL # default
)
``

This will optimise both the positions of the atoms and the cell geometry using sensible defaults for plane-wave cutoff, k-point meshes, etc.

Options for the `RelaxType`:

* `POSITIONS`: Only the atomic positions are relaxed, cell is fixed.
* `SHAPE`: Only the cell shape is optimized at a fixed volume and fixed atomic positions.
* `CELL`: Only the cell is optimized, both shape and volume, while atomic positions are fixed.
* `POSITIONS_SHAPE`: Same as `SHAPE`  but atomic positions are relaxed as well.
* `POSITIONS_CELL`: Same as `CELL`  but atomic positions are relaxed as well.


.. autodoc of PwRelaxWorkChain 

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants