search

siesta-project / aiida_siesta_plugin

Source code for the AiiDA-Siesta package (plugin and workflows). See wiki
Other
6 stars 11 forks source link

Flag to control MD.TypeOfRun when using LUA #122

Closed ahkole closed 1 year ago

ahkole commented 1 year ago

In my calculations I often use Lua scripts to do some constrained structural relaxations, where I still use the internal siesta relaxations algorithms (by setting MD.TypeOfRun to something other than Lua) and I use the Lua script to update certain internal siesta variables between relaxation steps. This was not possible with the current version of the AiiDA plugin because if you include a lua script it will always override MD.TypeOfRun and set it to Lua. Therefore, I added to flag to allow a user to change this behaviour. Maybe you could consider merging this into the plugin?

bosonie commented 1 year ago

Thanks @ahkole. Regarding the design, @albgar can comment. I will look at the technical details in the next few days (we have to make sure the change is fully back compatible and few other points). In the meanwhile, would you be able to add a small test for this new feature? Thanks!

ahkole commented 1 year ago

@bosonie I have added a small test to check the functionality of the control flag (i.e. to check that setting lua.md_run to False makes sure that the original value of mdtypeofrun is unchanged).

Also, the rest of the test suite still seems to pass without any issues :)

If you need me to add more tests for something or if you need anything else before a possible merge just let me know.

albgar commented 1 year ago

I think this is indeed appropriate. Just a small change in the documentation:

 * **lua.md_run** is a flag which controls whether we should set
-  ``MD.TypeOfRun`` to ``Lua``.
+  ``MD.TypeOfRun`` to ``Lua``. For most uses of Lua in geometry
+  relaxation and molecular dynamics, the default setting is
+  appropriate, but in some other cases one can still use Lua
+  profitably for more general tasks.
bosonie commented 1 year ago

Ok @ahkole. All good. I will merge this and I will later take care of the documentation change suggested by Alberto. Do you need me to make a release on PyPI? Thanks a million for your contribution.

ahkole commented 1 year ago

Ok @ahkole. All good. I will merge this and I will later take care of the documentation change suggested by Alberto.

Great, thanks!

Do you need me to make a release on PyPI?

I'm fine with using the development version from GitHub, so it's not strictly necessary for me. So if you want to wait to include more features/fixes in the next PyPI release that's fine for me.