Load tools automatically #69

pslacerda · 2016-06-29T04:07:51Z

Proposal for tool loading as in issue #68. Awaiting for opinions or changes proposals.

Even if almost complete it remains largely untested (eg. extra and groups).

- added logging at debug for the env setting (although not clear how to enable the logger at this stage unless it is started from another script which already has a gromacs logger running) - added entry to CHANGES

- checks for set_gmxrc_environment() moved inside function; Moved checking of input set_gmxrc_environment() into the function itself so that we can just safely call it from __init__ without further crud. (Also, the try/except OSError in __init__ for set_gmxrc_environment() would have never triggered because we were already catching it inside.) - use cfg.getpath() to get GMXRC from cfg file instead of cfg.get() - Added docs.

Conflicts: CHANGES

pslacerda · 2016-06-29T04:49:39Z

How can the following code of GromacsCommandMultiIndex works if ndxis a list?

# concatenate all index files
make_ndx = Make_ndx(f=kwargs.get('s'), n=ndx)
rc,out,err = make_ndx(o=self.multi_ndx, input=['q'], stdout=False, stderr=False)

Probably worked in some previous version. So can I assume that is safe to remove GromacsCommandMultiIndex?

pslacerda · 2016-07-01T02:25:03Z

Sorry, I was wrong. Of course it works!
Seems that now this PR is fully backward compatible. Please also take a look at merge_ndx().

pslacerda · 2016-07-02T05:04:24Z

Please look this PR! Of course it will not be accepted as is, it just has some ideas that may be useful.

For example load_v5_tools() ignoring the tools option because they can be fully automatically detected. Also I think that set_gmx_environment() must raise an error when the gmxrc argument value is invalid.

I apologize if my issues and commits seems disorganized because git just don't fit in my head! My local repo is messy and I do a lot of funny stuff to push a commit.

orbeckst · 2016-07-03T18:04:46Z

gromacs/tools.py

@@ -94,175 +71,202 @@ def _generate_sphinx_class_string(clsname):
    'mdrun': 'mdrun',
    'make_ndx': 'make_ndx',
    'make_edi': 'make_edi',
-    'gmxdump': 'gmxdump',
-    'gmxcheck': 'gmxcheck',
+    'dump': 'gmxdump',


This looks like a separate fix. Could you please make it a separate PR?

orbeckst · 2016-07-03T18:11:43Z

HI @pslacerda , can you please rebase to develop? During the 0.5.1 release #70 I had to rewrite the history of develop.

I haven't had time to do a full review of your PR (and I also want to see what is really new compared to develop). I like that you created more functions. Ping me when you're done, sorry, a bit busy at the moment!

Conflicts: CHANGES gromacs/__init__.py gromacs/config.py gromacs/tools.py

pslacerda · 2016-07-04T01:06:09Z

Of all that code seems that load_v5_tools() and merge_ndx() looks almost ready. The extra and tools options seems to work as expected. The suffix option was introduced to apply the suffix at v5 driver or v4 commands. If the tools option isn't provided, v4 guesses will be made from TOOLS_V4. I'm not sure if all v4 installations provides g_luck or if it's better to test mdrun or pdb2mgx. I'm also not sure if GromacsCommand.__doc__ is being properly set.

pslacerda · 2016-07-04T02:26:15Z

Now it has more adherence to the project's programming style. The driver or suffix option is needed to support double precision on Gromacs 5.x. Double precision isn't supported for 5.x, neither extraction of docstring by running commands with -h.

orbeckst · 2016-07-11T06:24:34Z

gromacs/config.py

-     load_tools.extend(cfg.getlist('Gromacs', g))
-del g
+
+def get_tools():


Good idea to make it a function.

pslacerda · 2016-07-28T01:23:11Z

ping @orbeckst,
All fixes were done. A review of configuration.txt will also improve my English writing.

pslacerda · 2016-07-28T01:28:45Z

What do you think about my previous suggestion of a mdp file dynamic repository. When the user know what he is doing is more a matter of join together mdp options and adjust parameters. Maybe we can help people at doing this. Molecular dynamics for noobs and repository for the rest, along with previously made GromacsWrapper programs we can seriously help to popularize molecular simulations.

pslacerda · 2016-07-28T01:36:01Z

Also the commits of this PR needs to be combined into one so don't mess with the repository history. And the patch-0.5.1 branch can already be deleted as was already merged into develop.

I'm not very skilled at git and github but seems relatively important things to do.

Following this PR I have some patches for core.py like this one, would be my last major patch at hand. I also propose move the plugins into a separated repository, will be cleaner and make easier to move just the main code to Python 3, for example.

pslacerda · 2016-07-28T01:38:53Z

Yes, seems that we're almost there with this PR!

pslacerda · 2016-07-28T01:43:20Z

Probably also remove or update the branch gh-pages
http://becksteinlab.github.io/GromacsWrapper/

orbeckst · 2016-08-01T18:09:49Z

@pslacerda – sorry for the delay... working on some other urgent stuff. Please ping again if nothing happened by Wednesday.

orbeckst · 2016-08-03T00:15:33Z

@pslacerda , quick comments while I have 5 minutes to spare:

For the discussion on an mdp repository, please open a new issue and flag it with the question tag (just copy and paste Load tools automatically #69 (comment) and Load tools automatically #69 (comment) ) – so that we can keep the discussion focused.
I also like the idea of splitting off the plugins Load tools automatically #69 (comment) and make the core library leaner. Please open a new issue.

Regarding this PR: I can do some rebasing and history massaging. I just need to test locally. I'll also look at the docs... in fact, I will probably get rid of the gh-pages branch, given that rtd http://gromacswrapper.readthedocs.io/ works for us – I opened #80.

orbeckst · 2016-08-07T17:59:11Z

Sorry @pslacerda – still not managed to try it. Busy with urgent stuff. Next week should be better.

orbeckst · 2016-08-16T06:51:04Z

With the new changes, one cannot install Gromacswrapper if Gromacs has not been sourced. See the output on travis job #102.1 and

(auto) yngvi:GromacsWrapper oliver$ pip install --upgrade .
Processing /Volumes/Data/oliver/Biop/Projects/Methods/GromacsWrapper
    Complete output from command python setup.py egg_info:
    Traceback (most recent call last):
      File "<string>", line 1, in <module>
      File "/var/folders/qs/k07z49fh8xl6xd008k8wxhxr0000gp/T/pip-YFccaC-build/setup.py", line 17, in <module>
        version = __import__('gromacs.version').get_version()
      File "gromacs/__init__.py", line 252, in <module>
        from . import tools
      File "gromacs/tools.py", line 296, in <module>
        registry = load_v5_tools()
      File "gromacs/tools.py", line 230, in load_v5_tools
        raise GromacsToolLoadingError("Failed to load v5 tools")
    gromacs.tools.GromacsToolLoadingError: Failed to load v5 tools

    ----------------------------------------
Command "python setup.py egg_info" failed with error code 1 in /var/folders/qs/k07z49fh8xl6xd008k8wxhxr0000gp/T/pip-YFccaC-build/

If I source GMXRC before installing then it works without problems.

However, we need to be able to install without Gromacs present.

…acsWrapper into automatic-tool-loading

pslacerda · 2016-08-16T23:12:37Z

I didn't had this problem here but now the setup.py doesn't import the whole package before install it, just import the version file.

pslacerda · 2016-08-16T23:52:18Z

I'm not understanding the errors on #103.1 as they doesn't happen here when running py.test gromacs. What do you think?

- make sure that MAJOR_RELEASE is empty if there is no cfg file - make sure that no tools are defined by default (otherwise autoloading will not look for more tools) - added debug logging to tool loading (activate with GW_START_LOGGING=true, see previous commit) - cbook: catch failed tool loading with AttributeError, too I think we can soon remove the template config file.

orbeckst · 2016-08-30T06:27:43Z

Sorry @pslacerda , this took forever. I ironed out a few bugs related to Gromacs v4 loading but seems to be good now. Merging now....

orbeckst · 2016-08-30T06:44:59Z

regarding errors on travis: not sure, @ianmkenney is working on tests for MDPOW and anything that he finds/cooks up will also be applicable to GW.

You can always raise an issue for tests that don't behave. I think there's already #61 (and #47).

pslacerda · 2016-08-31T01:30:46Z

Very nice @orbeckst! Was very rich experience commit on this project, I learned a lot about refactoring in Python. 😀

For sure I completly forgot about v4, thank you for fixing it.

What this line is doing? Doesn't it breaks the environment variables placing __ around it?

orbeckst · 2016-08-31T05:57:37Z

gromacs/config.py

    cmdargs = ['bash', '-c', ". {0} && echo {1}".format(gmxrc,
-               ' '.join(['${0}'.format(v) for v in envvars]))]
+               ' '.join(['___${{{0}}}___'.format(v) for v in envvars]))]


If a env var such as GMXPREFIX or GROMACS_DIR is not defined then it produces an empty value. Empty values are not picked up when the output is parsed and lead to wrong assignments of values to env vars. That was a problem with Gromacs 4.

Moving unknown env vars to the end would have been sufficient but using "Sentinel" markers around will ensure that even an empty result is picked up because it produces ___'___ (the ' should not be there but indicates where "nothing" is). The line below then removes the sentinels. Cludgy but fairly safe.

(Of course, everything just breaks if the env var values contain spaces as in GMXBIN=/Users/joe/Stuff for work/gromacs...)

Btw, it might be better to put the values on new lines and then split on those:

cmdargs = ['bash', '-c', ". {0} && echo {1}".format(gmxrc, '\n'.join(['${{{0}}}'.format(v) for v in envvars]))] ... out = out.split('\n') for key, value in zip(envvars, out): value = value.strip() os.environ[key] = value

This will work even for subsequent empty lines:

>>> out = "\n".join(["line 1", "line 2", "", "line 4", "", "", "line_5", ""]) >>> out.split("\n") ['line 1', 'line 2', '', 'line 4', '', '', 'line_5', '']

pslacerda · 2016-08-31T22:51:11Z

I got it!
It works now for every directory that do not contains __.

pslacerda · 2016-08-31T22:54:06Z

So lets put more __ around vars. I failed to use as separator one character unavailable in directory names (eg '\0', \1).

orbeckst · 2016-08-31T23:02:39Z

NULL string (\0) would be good (that's what find -print0 | xargs -0 ... uses) – I thought that no-one uses ___ in directory names but I am probably wrong...

Just fix develop, test it locally, and then commit to develop.

pslacerda and others added 4 commits June 8, 2016 22:04

Tries to detect and load GMXRC automatically (PR #55)

fe884a8

automatic environment settings from GMXRC (#55)

76c22ce

- added logging at debug for the env setting (although not clear how to enable the logger at this stage unless it is started from another script which already has a gromacs logger running) - added entry to CHANGES

Merge remote-tracking branch 'origin/feature-load-GMXRC' into develop

a96a432

Conflicts: CHANGES

pslacerda force-pushed the automatic-tool-loading branch 2 times, most recently from a0b0e51 to 9ec9e21 Compare June 29, 2016 05:00

orbeckst force-pushed the develop branch from a96a432 to 099c063 Compare June 29, 2016 20:46

Load tools automatically

9ec9e21

pslacerda added 3 commits July 1, 2016 23:20

new merge_ndx() function

b551c0c

minor improvements

0237b2b

tools config is optional

8370ec7

pslacerda force-pushed the automatic-tool-loading branch from df30326 to 8370ec7 Compare July 2, 2016 04:51

orbeckst reviewed Jul 3, 2016
View reviewed changes

pslacerda added 3 commits July 3, 2016 20:22

Merge branch 'develop' into automatic-tool-loading

9cb01ce

Conflicts: CHANGES gromacs/__init__.py gromacs/config.py gromacs/tools.py

Patching GromacsCommandMultiIndex only if tool at registry

73d3e38

Reverting things that don't belong this branch

fe0bfec

Code reduction

a2fc574

orbeckst reviewed Jul 11, 2016
View reviewed changes

gromacs/config.py

load_tools.extend(cfg.getlist('Gromacs', g))

del g

def get_tools():

Copy link

Member

orbeckst Jul 11, 2016

Choose a reason for hiding this comment

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

Good idea to make it a function.

stylistic fix

56db575

orbeckst mentioned this pull request Aug 3, 2016

install Gromacs in Travis CI #79

Closed

2 tasks

orbeckst modified the milestone: 0.6 Aug 3, 2016

Merge branch 'develop' into automatic-tool-loading

b47abe8

pslacerda added 2 commits August 16, 2016 20:09

not import the whole package while setuping

a90ff77

Merge branch 'automatic-tool-loading' of github.com:Becksteinlab/Grom…

29c81cb

…acsWrapper into automatic-tool-loading

orbeckst added 3 commits August 29, 2016 23:02

enable logging with envvar GW_START_LOGGING

52427d2

make GMXRC loading work for v4

e108934

orbeckst merged commit 1328b41 into develop Aug 30, 2016

orbeckst reviewed Aug 31, 2016
View reviewed changes

orbeckst deleted the automatic-tool-loading branch September 14, 2016 08:40

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Load tools automatically #69

Load tools automatically #69

pslacerda commented Jun 29, 2016 •

edited

pslacerda commented Jun 29, 2016

pslacerda commented Jul 1, 2016

pslacerda commented Jul 2, 2016

orbeckst Jul 3, 2016

pslacerda Jul 3, 2016

orbeckst commented Jul 3, 2016

pslacerda commented Jul 4, 2016

pslacerda commented Jul 4, 2016

orbeckst Jul 11, 2016

pslacerda commented Jul 28, 2016

pslacerda commented Jul 28, 2016

pslacerda commented Jul 28, 2016

pslacerda commented Jul 28, 2016

pslacerda commented Jul 28, 2016

orbeckst commented Aug 1, 2016

orbeckst commented Aug 3, 2016 •

edited

orbeckst commented Aug 7, 2016

orbeckst commented Aug 16, 2016

pslacerda commented Aug 16, 2016

pslacerda commented Aug 16, 2016

orbeckst commented Aug 30, 2016

orbeckst commented Aug 30, 2016

pslacerda commented Aug 31, 2016

orbeckst Aug 31, 2016

orbeckst Aug 31, 2016

pslacerda commented Aug 31, 2016

pslacerda commented Aug 31, 2016 •

edited

orbeckst commented Aug 31, 2016 •

edited

Load tools automatically #69

Load tools automatically #69

Conversation

pslacerda commented Jun 29, 2016 • edited

pslacerda commented Jun 29, 2016

pslacerda commented Jul 1, 2016

pslacerda commented Jul 2, 2016

orbeckst Jul 3, 2016

Choose a reason for hiding this comment

pslacerda Jul 3, 2016

Choose a reason for hiding this comment

orbeckst commented Jul 3, 2016

pslacerda commented Jul 4, 2016

pslacerda commented Jul 4, 2016

orbeckst Jul 11, 2016

Choose a reason for hiding this comment

pslacerda commented Jul 28, 2016

pslacerda commented Jul 28, 2016

pslacerda commented Jul 28, 2016

pslacerda commented Jul 28, 2016

pslacerda commented Jul 28, 2016

orbeckst commented Aug 1, 2016

orbeckst commented Aug 3, 2016 • edited

orbeckst commented Aug 7, 2016

orbeckst commented Aug 16, 2016

pslacerda commented Aug 16, 2016

pslacerda commented Aug 16, 2016

orbeckst commented Aug 30, 2016

orbeckst commented Aug 30, 2016

pslacerda commented Aug 31, 2016

orbeckst Aug 31, 2016

Choose a reason for hiding this comment

orbeckst Aug 31, 2016

Choose a reason for hiding this comment

pslacerda commented Aug 31, 2016

pslacerda commented Aug 31, 2016 • edited

orbeckst commented Aug 31, 2016 • edited

pslacerda commented Jun 29, 2016 •

edited

orbeckst commented Aug 3, 2016 •

edited

pslacerda commented Aug 31, 2016 •

edited

orbeckst commented Aug 31, 2016 •

edited