search

aiidateam / aiida-quantumespresso

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

First steps to make protocols scheduler agnostic #1011

Open t-reents opened 7 months ago

t-reents commented 7 months ago

Addresses #1010

The current protocols implicitly rely on the Slurm scheduler, thus causing errors when users try to use ad different one, e.g. SGE as described in the issue. Here, the documentation of the get_builder_from_protocol method is extended to make the users aware of this behavior. Moreover, a warning is added in case no explicit resources are provided via the options argument, as this will cause errors in future versions.

@sphuber @mbercx Suggestions for a different phrasing or place where to put the additional documentation are very welcome. Moreover, I used a UserWarning because a DeprecationWarning somehow didn't appear. I'm not sure whether they are filtered at some point?

sphuber commented 7 months ago

@sphuber @mbercx Suggestions for a different phrasing or place where to put the additional documentation are very welcome.

It would be great to also add this to the documentation in addition to the docstrings. I noticed that the get_builder_from_protocol functionality isn't really documented though.

Moreover, I used a UserWarning because a DeprecationWarning somehow didn't appear. I'm not sure whether they are filtered at some point?

Deprecation warnings are ignored by default in Python (https://docs.python.org/3/library/exceptions.html#DeprecationWarning). A user has to enable them manually to see them. I think a UserWarning is fine for now as an alternative.

t-reents commented 4 months ago

Hi @sphuber ! Sorry for picking this up just now.

I added a general description of the get_builder_from_protocol in the How to - Workflows section. To be honest, I wasn't 100% sure where to put it, but since it's a general remark about the workflows, I decided to put it there. Happy to discuss/adjust this.

Moreover, I realized that the issue of the protocols depending on SLURM also applies to the other workflows, not just the pw ones. Hence, I also added the warning to those.