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

Functions to plot networkx graphs of topology objects #492

Merged
merged 81 commits into from
May 11, 2021

Conversation

CalCraven
Copy link
Contributor

@CalCraven CalCraven commented Jan 10, 2021

This PR builds off of the convert_networkx PR.

This uses the to_networkx(topology) to plot networkx graphs of the topology. It is broken down into the ability to plot by-
atomtype: gmso.formats.interactive_networkx_atomtypes
bonds: gmso.formats.interactive_networkx_bonds
angles: gmso.formats.interactive_networkx_angles
dihedrals: gmso.formats.interactive_networkx_dihedrals

See the attached help strings for how to change the inputs to see different aspects of a topology.

I considered making these functions a method of the gmso.core.topology.Topology class. However, there are import issues. The gmso/external/convert_networkx.py file imports the topology class so it can create a topology from a networkx object. Since these functions make use of the to_networkx function in this file, either the to_networkx and from_networkx functions need to be separated, or we use this a separate file with these plot_networkx functions. I though it acceptable that they were part of the gmso/formats folder, since networkx is another potential way to output information about these structures.

I'm hoping to continue to add some interactive functionality for working with these networkx objects. This will use ipywidget.interact.

@CalCraven CalCraven added enhancement New feature or request visualization labels Jan 10, 2021
@codecov
Copy link

codecov bot commented Jan 10, 2021

Codecov Report

Merging #492 (9f3ad31) into master (893534b) will increase coverage by 0.25%.
The diff coverage is 92.28%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #492      +/-   ##
==========================================
+ Coverage   89.77%   90.02%   +0.25%     
==========================================
  Files          50       51       +1     
  Lines        3560     3910     +350     
==========================================
+ Hits         3196     3520     +324     
- Misses        364      390      +26     
Impacted Files Coverage Δ
gmso/core/topology.py 97.42% <ø> (ø)
gmso/utils/nx_utils.py 92.16% <92.16%> (ø)
gmso/utils/io.py 96.77% <94.11%> (-1.01%) ⬇️
gmso/formats/__init__.py 100.00% <100.00%> (ø)
gmso/core/angle.py 100.00% <0.00%> (+3.84%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 893534b...9f3ad31. Read the comment docs.

@lgtm-com
Copy link
Contributor

lgtm-com bot commented Jan 10, 2021

This pull request introduces 6 alerts when merging 905263e into 49217ca - view on LGTM.com

new alerts:

  • 3 for Unused import
  • 2 for Unused local variable
  • 1 for Testing equality to None

@lgtm-com
Copy link
Contributor

lgtm-com bot commented Jan 14, 2021

This pull request introduces 6 alerts when merging 3d6e35b into 49217ca - view on LGTM.com

new alerts:

  • 3 for Testing equality to None
  • 2 for Unused import
  • 1 for 'import *' may pollute namespace

…is_atype

"merge new methods to convert angles and dihedrals from the _get_angles,dihderals_for methods in topology.py"
@lgtm-com
Copy link
Contributor

lgtm-com bot commented Jan 15, 2021

This pull request introduces 6 alerts when merging c1ff20f into 49217ca - view on LGTM.com

new alerts:

  • 3 for Testing equality to None
  • 2 for Unused import
  • 1 for 'import *' may pollute namespace

@lgtm-com
Copy link
Contributor

lgtm-com bot commented Jan 20, 2021

This pull request introduces 3 alerts when merging b573ebe into 49217ca - view on LGTM.com

new alerts:

  • 3 for Testing equality to None

@lgtm-com
Copy link
Contributor

lgtm-com bot commented Jan 21, 2021

This pull request introduces 3 alerts when merging 9a4d40c into 49217ca - view on LGTM.com

new alerts:

  • 3 for Testing equality to None

@lgtm-com
Copy link
Contributor

lgtm-com bot commented Jan 28, 2021

This pull request introduces 2 alerts when merging cc07db2 into cbb547d - view on LGTM.com

new alerts:

  • 1 for Unused local variable
  • 1 for Unused import

@lgtm-com
Copy link
Contributor

lgtm-com bot commented Feb 4, 2021

This pull request introduces 2 alerts when merging b083f03 into 55e634d - view on LGTM.com

new alerts:

  • 1 for Unused local variable
  • 1 for Unused import

CalCraven and others added 2 commits April 8, 2021 10:47
Co-authored-by: Umesh Timalsina <umesh.timalsina@vanderbilt.edu>
@jennyfothergill
Copy link
Collaborator

Great work :) I feel like this PR could be adapted to also address these issues:
mosdef-hub/foyer#287 - visualize atomtyping
mosdef-hub/foyer#258 - add readme image (your visualization would be great here, see mwthompsons draft image)
mosdef-hub/foyer#169 - report unparameterized angles, etc
mosdef-hub/foyer#323 - roughly the same as 169

@CalCraven
Copy link
Contributor Author

CalCraven commented Apr 15, 2021

mosdef-hub/foyer#287 - visualize atomtyping

I would say this PR covers this use case. Is there a way to close the issue by linking to this PR.

mosdef-hub/foyer#258 - add readme image (your visualization would be great here, see mwthompsons draft image)

I can take a sample system and add the visualization to the README for Foyer or GMSO. Honestly, I think it might be useful to have a single image that goes on both README's, that shows the connection between the two packages.

mosdef-hub/foyer#169 - report unparameterized angles, etc

This also addresses this issue! If you plot using networkx bonds, angles, or dihedrals and don't specify which atoms you're interested in getting a list of parameters for, the unparameterized connections will be shown.

mosdef-hub/foyer#323 - roughly the same as 169

Again, these needs should be addressed with this PR.

@CalCraven CalCraven self-assigned this Apr 26, 2021
Copy link
Contributor Author

@CalCraven CalCraven left a comment

Choose a reason for hiding this comment

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

Umesh reviews addressed

@lgtm-com
Copy link
Contributor

lgtm-com bot commented Apr 29, 2021

This pull request introduces 1 alert when merging 813f605 into 893534b - view on LGTM.com

new alerts:

  • 1 for Unused import

@CalCraven CalCraven mentioned this pull request May 3, 2021
@lgtm-com
Copy link
Contributor

lgtm-com bot commented May 3, 2021

This pull request introduces 1 alert when merging 494553d into 893534b - view on LGTM.com

new alerts:

  • 1 for Syntax error

Copy link
Contributor Author

@CalCraven CalCraven left a comment

Choose a reason for hiding this comment

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

Applied Umesh suggestions.

@CalCraven CalCraven merged commit 3e5b853 into mosdef-hub:master May 11, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request visualization
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants