[WIP] Added an example driver for dynamics, do NOT merge

[zerothi]

This is meant as an exercise on adding new motion classes (it SHOULDN'T be merged).

I have checked it works with Siesta and I can retrieve and send forces/coordinates.

These are notes taken during the development of this PR. The PR is not meant as an actual PR but rather to inform the developers of problems related to the development of new motion classes.

Here are my "random" thoughts while developing a new Motion class. Please do not take them as critisicms!

1. When adding a motion class it is unclear which methods are required for a fully functioning motion class.

2. The dependency scheme is smart, but since the documentation of this is low it is extremely difficult to figure out how to structure the new Motion class. I.e. a call-graph with method names would be really helpful. Fig. 1.2 and 1.3 seems to document the dependency scheme. But not for developers.

   Also, in the dependency scheme how does it now that new values have entered? I.e. when is the cached value reset and how does ipi track this information?

3. It seems that adding a new motion class also requires a new InputMotion class to accompany it (if variable inputs are required). The structure of the Input* class is also un-documented.

   3a) An idea, would it make sense to automatically populate the .fields in the Input* classes by looping and checking their inheritance? Currently this does not work because the InputDynamics is not sub-classed from InputMotionBase/InputMotion, but I guess some kind of higher class based scheme could be used to automate some of the lists?

4. Input* classes and their corresponding engine classes seems to be highly connected. The engines are passed as arguments to the Input.store method (something that is not entirely clear from the documentation). But it seems to me that the input class should feed data to the engine class (is this done in Simulation?). This seems a bit backwards at a first glance. I would suspect input to return a simulation class, ready to be used?

5. When running a calculation there is very little help if the input.xml is badly formatted. I.e. you would simply get an error message like this:

   File "/opt/python/2.7.14/gnu-7.3.0/bin/i-pi", line 4, in import('pkg_resources').run_script('i-PI==1.0', 'i-pi') File "build/bdist.linux-x86_64/egg/pkg_resources/init.py", line 750, in run_script File "build/bdist.linux-x86_64/egg/pkg_resources/init.py", line 1527, in run_script File "/opt/python/2.7.14/gnu-7.3.0/lib/python2.7/site-packages/i_PI-1.0-py2.7.egg/EGG-INFO/scripts/i-pi", line 101, in main(args[0], options.do_yappi) File "/opt/python/2.7.14/gnu-7.3.0/lib/python2.7/site-packages/i_PI-1.0-py2.7.egg/EGG-INFO/scripts/i-pi", line 48, in main simulation = Simulation.load_from_xml(fn_input, request_banner=True) File "/opt/python/2.7.14/gnu-7.3.0/lib/python2.7/site-packages/i_PI-1.0-py2.7.egg/ipi/engine/simulation.py", line 78, in load_from_xml input_simulation.parse(xmlrestart.fields[0][1]) File "/opt/python/2.7.14/gnu-7.3.0/lib/python2.7/site-packages/i_PI-1.0-py2.7.egg/ipi/utils/inputvalue.py", line 338, in parse self.extend(f, v) File "/opt/python/2.7.14/gnu-7.3.0/lib/python2.7/site-packages/i_PI-1.0-py2.7.egg/ipi/utils/inputvalue.py", line 236, in extend raise ValueError("Error parsing " + name + " from " + str(xml)) ValueError: Error parsing system from <ipi.utils.io.inputs.io_xml.xml_node object at 0x2b0c1bf2bf50>

   This small change would probably help (a little):

   diff --git a/ipi/utils/inputvalue.py b/ipi/utils/inputvalue.py index 4e31e63c..c5c22b84 100644 --- a/ipi/utils/inputvalue.py +++ b/ipi/utils/inputvalue.py @@ -232,7 +232,10 @@ class Input(object): try: newfield = self.dynamic[name]0 newfield.parse(xml)

   • except:
   • except Exception as e:
   • print(e)
   • print('XML.node: {0}'.format(xml.name))
   • print(' attribs: {0}'.format(xml.attribs)) raise ValueError("Error parsing " + name + " from " + str(xml)) self.extra.append((name, newfield))

   however an extended help would be even better (for instance all variables have a help in their Input* definitions, so one could print out these in case of an error?

6. The unit only allow atomic_unit, however, wouldn't it make sense to allow Bohr as a unit-specification? Since Angstrom is an allowed unit-specification.

7. Lastly, while all this development is not too difficult (once the initial barrier has been hurdled) it would be very nice if one could make i-PI import plugins. I.e. something like

   this would allow a fixed i-PI installation and allow easier hooks for external use. There are several choices regarding plugins, but this was just a basic idea.

[ceriottm]

Any "chirurgical" PR that solves a small issue is welcome - e.g. those you made earlier on documentation not compiling, and adding a command-line switch for verbosity. Major overhaul will have to wait. The depend machinery is actually explained quite decently in the CPC paper and in the manual. Basically you have to define in a declarative style how different quantities depend on each other when you first introduce them (typically in the init or bind routines of a module). Then it automatically tracks dependencies and caching, e.g. if you require forces.f it's computed and cached, and re-computed only if the cell or the positions are touched.

[zerothi]

I don't think this should ever enter the code base. It was more of a documentation for the steps required to implement ones own motion class. So it may serve as an example for new developers.

The depend machinery is explained in the manual, but as far as I can see there are no code examples (which is what I was after).

[mahrossi]

Thanks Nick. I think many suggestions that you make are very helpful and they also really point to the fact that the documentation in general can be made better, and we agree. Hopefully the documentation that we will release with i-PI v2 will be a bit more comprehensive. One side-note on the dependency scheme that may not be clear from Michele's phrase and has generated confusion when we started implementing stuff in i-PI: forces.f is only recomputed if cell or positions are touched and only when it is called again from the code (may be obvious to many people but was not obvious to at least 4 people over here :) ).

Specifically, point 5 would be a simple fix that would help many users. Then I don't understand what you mean in point 7. Can you clarify? What do you mean by only atomic_unit is allowed? you just mean you want to call it bohr instead?

On Fri, Apr 13, 2018 at 1:54 PM, Nick R. Papior notifications@github.com wrote:

I don't think this should ever enter the code base. It was more of a documentation for the steps required to implement ones own motion class. So it may serve as an example for new developers.

The depend machinery is explained in the manual, but as far as I can see there are no code examples (which is what I was after).

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/cosmo-epfl/i-pi-dev/pull/235#issuecomment-381112479, or mute the thread https://github.com/notifications/unsubscribe-auth/AGbkQmH3F_xxvj9V-hKzpWx6FfYTuuwyks5toJHcgaJpZM4TTRZA .

[zerothi]

@ceriottm, @mahrossi Thanks for your interest in my feedback.

As for 6., it was just that some may prefer Bohr over atomic_unit (or at least be more comfortable with that unit), I would simply allow both unit-specifications as the same thing. Not only Bohr.