Conversation
…h LUA step created for restarting the NEB calculation
adding tutorial folder
|
How are things going? ;) I really would like to get back to this. I'll probably have to post-pone until after the transiesta workshop, but could you please provide some more details. And later, it would be nice if pull requests were a bit smaller and more divided (then it is easier to review them ;)) Thanks again!!! |
zerothi
left a comment
There was a problem hiding this comment.
Lets discuss this Wednesday.
It really would help to separate things a bit. The different bits gets bloated and it is not easy for me to see where stuff belongs.
Also, I have to think about the documentation, since lua does not intrinsically support sphinx, other tools are currently more favoured, so I have to consider heavily what to do here...
| from ase import Atoms | ||
| from ase.neb import NEB | ||
| import os, shutil, sys | ||
| from linecache import getline |
| parser = argparse.ArgumentParser() | ||
| parser.add_argument('-d', "--directory", type=str, default=".", | ||
| help='Directory where the fdf files are located, default is the current directory') | ||
| parser.add_argument('-n', "--nimages", type=int, default=5, |
There was a problem hiding this comment.
I would just use -n and --images it should be clear that it refers to number of.
| help='Directory where the fdf files are located, default is the current directory') | ||
| parser.add_argument('-n', "--nimages", type=int, default=5, | ||
| help='Number of images that must be generated between the two states, default is 5') | ||
| parser.add_argument('-if', "--initialfile", type=str, default="initial", |
| help='Number of images that must be generated between the two states, default is 5') | ||
| parser.add_argument('-if', "--initialfile", type=str, default="initial", | ||
| help='Name of the initial structure file (without extension), default is "initial"') | ||
| parser.add_argument('-ff', "--finalfile", type=str, default="final", |
| help='Name of the initial structure file (without extension), default is "initial"') | ||
| parser.add_argument('-ff', "--finalfile", type=str, default="final", | ||
| help='Name of the final structure file (without extension), default is "final"') | ||
| parser.add_argument('-m', "--method", type=str, default="li", |
There was a problem hiding this comment.
spell it out, also, probably use choices=('linear') to make it clear what they mean li can mean anything (almost) ;)
|
|
||
| end | ||
|
|
||
| function DNEB:file_exists(name)--name |
There was a problem hiding this comment.
Also, some of these should be needed? No, if you use inheritance all should be good?
| end | ||
|
|
||
|
|
||
| function NEB:file_exists(name)--name |
There was a problem hiding this comment.
This shouldn't be a class method here. This looks more like a utility?
| end | ||
| return NEB_F | ||
| end | ||
| --- Query the current force (same as `NEB:force` but with IO included) |
There was a problem hiding this comment.
Space to separate methods etc. Also looks like indentation problems?
| @@ -0,0 +1,1821 @@ | |||
| O ca nrl nc | |||
There was a problem hiding this comment.
I don't think we should mix these things here... This is better suited for Siesta.
| @@ -0,0 +1,124 @@ | |||
| --[[ | |||
There was a problem hiding this comment.
You are mixing many things at once. It is easier for me to see what belongs where if we could have separated these things.
1 for tutorials, 1 for NEB stuff and 1 for documentation
Dear Nick
Hi i hope i done everything right!... plz just don't merge it yet I'm still doing changes ... i just open this pull for u to check things are in correct path
Cheers