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

mdacli doesn't loop over single frames #98

Closed
hejamu opened this issue Apr 21, 2022 · 2 comments
Closed

mdacli doesn't loop over single frames #98

hejamu opened this issue Apr 21, 2022 · 2 comments

Comments

@hejamu
Copy link
Collaborator

hejamu commented Apr 21, 2022

If one is using an AnalysisClass on a trajectory with only one frame, the AnalysisBaseClass from MDA will happily do exactly that. This is because at initialization (
https://github.com/MDAnalysis/mdanalysis/blob/3769ee29e5907221527ff0ec88a8c5acf9f86dee/package/MDAnalysis/analysis/base.py#L247
), if start, stop and step are None, and the trajectory is ONE frame long, it sets them to 0, 1 and 1 respectively.

mdacli however sets the default values to start = 0, stop = -1 and step = 1. This does not result in the same behaviour, since for array of length 1:

a[0:-1:1] != a[0:1:1]

This difference in behaviour finally matters at this point: https://github.com/MDAnalysis/mdanalysis/blob/3769ee29e5907221527ff0ec88a8c5acf9f86dee/package/MDAnalysis/analysis/base.py#L296

Maybe setting the defaults to None would be an option?

@PicoCentauri
Copy link
Collaborator

Good idea of changing these. Would you be able to do it? The lines are here:

mdacli/src/mdacli/libcli.py

Lines 182 to 204 in cd90206

run_group.add_argument(
"-b",
dest="start",
type=str,
default="0",
help="start frame or time for evaluation (default: %(default)s)"
)
run_group.add_argument(
"-e",
dest="stop",
type=str,
default="-1",
help="end frame or time for evaluation (default: %(default)s)"
)
run_group.add_argument(
"-dt",
dest="step",
type=str,
default="1",
help="step or time step for evaluation (default: %(default)s)"
)

We ran through a test to check the units and maybe the function

def split_time_unit(s):

will breaks. An additional test for None would be sufficient I guess.

@PicoCentauri
Copy link
Collaborator

Closed by #99

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

No branches or pull requests

2 participants