Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow parser-specific command-line options which get passed to parser to handle internally #47

Closed
halx opened this issue Dec 13, 2015 · 10 comments

Comments

@halx
Copy link
Collaborator

halx commented Dec 13, 2015

It should be possible for the user to set parser specific options, e.g. from the command line like -a AMBER:write_grads:verbose=3:do_something_else. The main code will split this in a list and pass it on to the parser code.

@davidlmobley
Copy link
Member

Can you be more specific about what problem(s) you see this solving and what you think should be implemented? I'm not necessarily opposed - I'm just not totally clear as to what you're getting at.

@halx
Copy link
Collaborator Author

halx commented Dec 17, 2015

This is as simple as

tmp = software.split(':')
software = tmp[0]
parser_opts = tmp[1:]

and pass parser_opts to the parser. The parser makes use of this at it
seems fit.

MD codes have different output and features and it is useful to extract
some of those and handle them with the parser. E.g. AMBER collects
averages of the components. This is not vital to the code but can be
informative. So an option could be there to write output (also to a file)
whent the user wants it. Another option would be to write out all the
parsed gradients into a new file. GROMACS has a nice format (XVG) but not
so in AMBER and possibly other codes. Some codes may write out, say,
gradients very frequently e.g. every step which can be a problem with
memory so a skip options (or whatever) can come quite in handy.

Of course, these examples may be useful for all the parsers and could be
implemented as a global options list. BTW, I think it would be useful too
to write out (again as an option) the uncorrelated gradient data. While
the plots as done in the code now are nice a user would have full
flexibility to handle these data in whatever fashion they wish.

On 16 December 2015 at 22:23, David L. Mobley [email protected]
wrote:

Can you be more specific about what problem(s) you see this solving and
what you think should be implemented? I'm not necessarily opposed - I'm
just not totally clear as to what you're getting at.


Reply to this email directly or view it on GitHub
#47 (comment)
.

@davidlmobley
Copy link
Member

OK, with this clarification I agree this is a good idea. It should be coupled to the standardization of the parser interface we've been talking about elsewhere (i.e. under #48 ).

@davidlmobley davidlmobley changed the title Parser specific options Allow parser-specific command-line options which get passed to parser to handle internally Dec 22, 2015
@davidlmobley
Copy link
Member

Also, I want to remark that we want to make sure to do this in a way that the options are just passed to the parser directly to handle internally, so that the code outside doesn't need to know about them.

@halx
Copy link
Collaborator Author

halx commented Jan 18, 2016

Done, although some functionality may be useful globally.

@halx halx closed this as completed Jan 18, 2016
@davidlmobley
Copy link
Member

Er, where was this done? Sorry, I'm just confused.

@halx
Copy link
Collaborator Author

halx commented Jan 19, 2016

In the refactor branch. I am currently not introducing any changes to my
fork and certainly not to the master branch.

On 19 January 2016 at 00:27, David L. Mobley [email protected]
wrote:

Er, where was this done? Sorry, I'm just confused.


Reply to this email directly or view it on GitHub
#47 (comment)
.

@davidlmobley
Copy link
Member

OK, right. Do you mind submitting a PR from your "refactor" branch to the master branch here (but obviously not merging) to make it easier to:
a) see what you're doing, and
b) allow issues that are closed by those revisions to reference the PR, and
c) discuss your changes

Usually we do this (elsewhere) by submitting a PR with the characters "[WIP]" as the first characters in the title. That stands for "work in progress" and basically means it's just there to show people what you're doing and NOT yet ready for merging. It provides a good opportunity for discussion and comments on your changes, etc. For example, see choderalab/openmoltools#164.

In case it's not obvious - changes you push to your branch after submitting the PR will automatically go into updates to the PR, so you basically just leave it open and keep making updates until you think it's ready to merge, then you edit the title to remove the "[WIP]" from it.

@halx
Copy link
Collaborator Author

halx commented Jan 21, 2016

Done.

On 20 January 2016 at 19:59, David L. Mobley [email protected]
wrote:

OK, right. Do you mind submitting a PR from your "refactor" branch to the
master branch here (but obviously not merging) to make it easier to:
a) see what you're doing, and
b) allow issues that are closed by those revisions to reference the PR

Usually we do this (elsewhere) by submitting a PR with the characters
"[WIP]" as the first characters in the title. That stands for "work in
progress" and basically means it's just there to show people what you're
doing and NOT yet ready for merging. It provides a good opportunity for
discussion and comments on your changes, etc. For example, see
choderalab/openmoltools#164
choderalab/openmoltools#164.

In case it's not obvious - changes you push to your branch after
submitting the PR will automatically go into updates to the PR, so you
basically just leave it open and keep making updates until you think it's
ready to merge, then you edit the title to remove the "[WIP]" from it.


Reply to this email directly or view it on GitHub
#47 (comment)
.

@davidlmobley
Copy link
Member

Thanks, I got the notification and will start looking at code hopefully
soon (still way backlogged on this end).

DM

On Thu, Jan 21, 2016 at 1:09 AM, Hannes Loeffler [email protected]
wrote:

Done.

On 20 January 2016 at 19:59, David L. Mobley [email protected]
wrote:

OK, right. Do you mind submitting a PR from your "refactor" branch to the
master branch here (but obviously not merging) to make it easier to:
a) see what you're doing, and
b) allow issues that are closed by those revisions to reference the PR

Usually we do this (elsewhere) by submitting a PR with the characters
"[WIP]" as the first characters in the title. That stands for "work in
progress" and basically means it's just there to show people what you're
doing and NOT yet ready for merging. It provides a good opportunity for
discussion and comments on your changes, etc. For example, see
choderalab/openmoltools#164
choderalab/openmoltools#164.

In case it's not obvious - changes you push to your branch after
submitting the PR will automatically go into updates to the PR, so you
basically just leave it open and keep making updates until you think it's
ready to merge, then you edit the title to remove the "[WIP]" from it.


Reply to this email directly or view it on GitHub
<
#47 (comment)

.


Reply to this email directly or view it on GitHub
#47 (comment)
.

David Mobley
[email protected]
949-385-2436

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants