Project

General

Profile

Bug #1742

Unexpected behavior and seg. fault for gmx select

Added by Viveca Lindahl about 2 years ago. Updated about 2 years ago.

Status:
Closed
Priority:
Normal
Assignee:
Category:
selections
Target version:
Affected version - extra info:
4.5.*, 4.6.*, 5.0-5.0.5
Affected version:
Difficulty:
uncategorized
Close

Description

1)

gmx select -select '(resname SOL and atomnr 1)' -on

produces the error

'Error in user input:
Selection '(resname SOL and atomnr 1)' never matches any atoms.'

if atom 1 is not a SOL. In turn,

gmx select -select '(same residue as (resname SOL and atomnr 1))' -on

produces a segmentation fault which makes it difficult to spot the original problem.

2) Another surprise:

Assuming the first SOl has atomnr 52345,

gmx select -select '(resname SOL and atomnr 1 to 52345)' -on

yields an index file of 1 atom as expected, while

gmx select -select '(resname SOL and atomnr 52346 to 52345)' -on

does not yield an error (but an index file with atoms 52345, 52346) although there is probably something wrong.

Associated revisions

Revision fe649c41 (diff)
Added by Teemu Murtola about 2 years ago

Add note about atom order in selections

Clarify the behavior of selections in that they always select atoms in
increasing order. In the same context, mention the mechanisms in place
to work around this. Use an example that shows the equivalence of
'a to b' and 'b to a' in the selection syntax.

Also clean up the formatting of the selection limitations help topic,
now that bullet lists are properly supported.

Related to #1742

Change-Id: I3daa17521767b3eaadcbc1d8c0c6fc986fa0aee3

Revision 1cdc52e9 (diff)
Added by Teemu Murtola about 2 years ago

Avoid crash with empty reference for 'same as' selection

Make 'same ... as none' selections not segfault. If there were no atoms
in the reference group, the code that sorted the values and removed
duplicates incorrectly set the number of values as one, resulting in
incorrect memory access later.

Fixes #1742 (segfault part).

Change-Id: I9d14c30404121356ee3abf1a5575fb0baa82fb7b

History

#1 Updated by Teemu Murtola about 2 years ago

  • Category set to selections
  • Status changed from New to Accepted
  • Assignee set to Teemu Murtola

The segfault is easily reproducible; I'll try to fix it.

It is intentional that "a to b" is interpreted the same as "b to a", no matter what a and b are. What would you expect to happen in the latter case? What would be a reasonable logic to detect that error that would not prevent reasonable input?

#2 Updated by Viveca Lindahl about 2 years ago

I wouldn't expect "a to b to be symmetric but I would expect a < b. Perhaps this is my misunderstanding and not actually incorrect behavior.

#3 Updated by Gerrit Code Review Bot about 2 years ago

Gerrit received a related patchset '1' for Issue #1742.
Uploader: Teemu Murtola ()
Change-Id: I9d14c30404121356ee3abf1a5575fb0baa82fb7b
Gerrit URL: https://gerrit.gromacs.org/4638

#4 Updated by Teemu Murtola about 2 years ago

  • Status changed from Accepted to Fix uploaded
  • Target version set to 5.0.6

Fix for the segfault is in Gerrit.

For the behavior of a to b, I guess that a case can be made for both behaviors, but I still think that it is reasonable to expect that, e.g., charge {-0.7 to -1} to be a valid selection. At minimum, I'll add a note about this to the documentation (possibly only for 5.1 onwards), but other suggestions are also welcome.

#5 Updated by Gerrit Code Review Bot about 2 years ago

Gerrit received a related patchset '1' for Issue #1742.
Uploader: Teemu Murtola ()
Change-Id: I3daa17521767b3eaadcbc1d8c0c6fc986fa0aee3
Gerrit URL: https://gerrit.gromacs.org/4640

#6 Updated by Teemu Murtola about 2 years ago

The second change updates the selection documentation to be clearer on the atom order and the behavior of a to b, partially prompted by this issue.

#7 Updated by Teemu Murtola about 2 years ago

  • Affected version - extra info set to 4.5.*, 4.6.*, 5.0-5.0.5

#8 Updated by Teemu Murtola about 2 years ago

  • Status changed from Fix uploaded to Resolved

#9 Updated by Teemu Murtola about 2 years ago

  • Status changed from Resolved to Closed

Also available in: Atom PDF