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

Emit warnings for empty Ag and user grid in DensityAnalysis #3091

Merged
merged 36 commits into from
Mar 11, 2021
Merged
Show file tree
Hide file tree
Changes from 11 commits
Commits
Show all changes
36 commits
Select commit Hold shift + click to select a range
fb491fd
Update topologyattrs.py
aditya-kamath Jan 1, 2021
7fec795
Changed-authors,changelog
aditya-kamath Jan 2, 2021
c965b77
including Tests for atommethods Quickfix
aditya-kamath Jan 2, 2021
d2d54fc
minor correction to test_topologyattrs.py
aditya-kamath Jan 2, 2021
2b1c50d
update - test_topologyattrs.py
aditya-kamath Jan 2, 2021
c00e3f3
Fixing PEP8 issues
aditya-kamath Jan 3, 2021
65e0c45
add name to CHANGELOG
aditya-kamath Jan 3, 2021
14acdfb
Fixing PEP8 indent error
aditya-kamath Jan 4, 2021
d0f5263
Attempt to resolve empty selection
aditya-kamath Jan 6, 2021
4257b45
Update CHANGELOG
aditya-kamath Jan 6, 2021
e4fc9aa
Merge branch 'develop' into develop
aditya-kamath Jan 7, 2021
8232192
Merge remote-tracking branch 'upstream/develop' into develop
aditya-kamath Jan 7, 2021
52fd1f3
reducing abstraction in density.py
aditya-kamath Jan 9, 2021
eff2854
Merge branch 'develop' of https://github.com/aditya-kamath/mdanalysis…
aditya-kamath Jan 9, 2021
ec9c1f3
Revert "reducing abstraction in density.py"
aditya-kamath Jan 9, 2021
f4319ee
Revert "Revert "reducing abstraction in density.py""
aditya-kamath Jan 9, 2021
0423a48
New approach to solve empty atomgroup in densityanalysis
aditya-kamath Jan 17, 2021
c7caaef
Minor change to \ break line
aditya-kamath Jan 17, 2021
b861b34
Fixing autopep8 errors and review changes
aditya-kamath Jan 18, 2021
d6d23d6
Changed to raise error when no user-defined grid present
aditya-kamath Jan 19, 2021
33c94fa
Minor corrections and test added for no atomgroup fail.
aditya-kamath Jan 19, 2021
dab9ad9
Minor PEP8 corrections
aditya-kamath Jan 19, 2021
7c22dc1
User friendly messages
aditya-kamath Jan 20, 2021
dc834c8
Docstring+warning message change
aditya-kamath Feb 3, 2021
4e8008e
PEP8 Fixs
aditya-kamath Feb 3, 2021
a83c27b
fix PEP8 errors
aditya-kamath Feb 7, 2021
6675097
Merge branch 'develop' into develop
aditya-kamath Feb 8, 2021
d22916c
Update Docstring
aditya-kamath Feb 11, 2021
f3af280
Revert cosmetic changes
aditya-kamath Feb 14, 2021
245a859
fix PEP8 issues
aditya-kamath Feb 15, 2021
9c127f5
Update Docstring +changelog
aditya-kamath Feb 15, 2021
e652669
Fixing PEP8 issues
aditya-kamath Feb 17, 2021
e3990ee
orbeckst requested changes
aditya-kamath Feb 20, 2021
5a2b371
Adding a new method to docs
aditya-kamath Feb 21, 2021
10db351
updates to fix doc fail
aditya-kamath Feb 26, 2021
9341947
revert change to line not covered in tests
aditya-kamath Mar 3, 2021
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions package/CHANGELOG
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ The rules for this file:
* 2.0.0

Fixes
* UserWarning displayed when empty atomgroup is given to DensityAnalysis (Issue #3055)
aditya-kamath marked this conversation as resolved.
Show resolved Hide resolved
* atommethods (_get_prev/next_residues_by_resid) returns empty residue group
when given empty residue group (Issue #3089)
* MOL2 files without bonds can now be read and written (Issue #3057)
Expand Down
15 changes: 15 additions & 0 deletions package/MDAnalysis/analysis/density.py
Original file line number Diff line number Diff line change
Expand Up @@ -384,6 +384,11 @@ def __init__(self, atomgroup, delta=1.0,
self._zdim = zdim

def _prepare(self):
if len(self._atomgroup)==0:
msg = ("No atoms in selection over whole trajectory")
aditya-kamath marked this conversation as resolved.
Show resolved Hide resolved
warnings.warn(msg)
logger.warning(msg)
return
coord = self._atomgroup.positions
if self._gridcenter is not None:
# Issue 2372: padding is ignored, defaults to 2.0 therefore warn
Expand Down Expand Up @@ -424,6 +429,11 @@ def _prepare(self):
self.density = None

def _single_frame(self):
if len(self._atomgroup)==0:
msg = ("No atoms in selection over whole trajectory")
warnings.warn(msg)
logger.warning(msg)
return
h, _ = np.histogramdd(self._atomgroup.positions,
bins=self._bins, range=self._arange,
normed=False)
Expand All @@ -434,6 +444,11 @@ def _single_frame(self):
self._grid += h

def _conclude(self):
if len(self._atomgroup)==0:
msg = ("No atoms in selection over whole trajectory")
warnings.warn(msg)
logger.warning(msg)
return
Copy link
Member

Choose a reason for hiding this comment

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

I wonder if three duplications of the same code block is enough to justify abstraction to a single i.e., function. Maybe borderline if the return has to remain outside of the abstraction I suppose.

On a more basic level, just asking--the code will still produce the original failure/exception (is that what it was doing?) and this will simply add a warning to clarify?

Copy link
Contributor Author

@aditya-kamath aditya-kamath Jan 7, 2021

Choose a reason for hiding this comment

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

This might sound very primitive but my logic was to return out of the function and warn the user of no atoms until atomgroup gets updated with atleast one atom. In the Issue, the user wanted to avoid the program crashing and would provide a dummy atom to keep the density frames generating. I tried to fix this by preventing a crash at the initializing, frame generation and conclusion. Perhaps a more efficient way is to make a function to check for no atoms and return.

Copy link
Contributor Author

@aditya-kamath aditya-kamath Jan 7, 2021

Choose a reason for hiding this comment

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

Again, my understanding of the DensityAnalysis function process may not be very accurate so I apologize for any incorrect arguments I make :P

# average:
self._grid /= float(self.n_frames)
density = Density(grid=self._grid, edges=self._edges,
Expand Down