-
Notifications
You must be signed in to change notification settings - Fork 34
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
willard_chandler.py #332
willard_chandler.py #332
Conversation
pytim/patches.py
Outdated
def patchMDTRAJ_ReplacementTables(): | ||
try: | ||
import mdtraj | ||
if not (mdtraj.formats.pdb.PDBTrajectoryFile._atomNameReplacements=={}): | ||
if not (mdtraj.formats.pdb.PDBTrajectoryFile._atomNameReplacements == {}): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
line too long (82 > 79 characters)
pytim/__init__.py
Outdated
from .willard_chandler import WillardChandler | ||
from .chacon_tarazona import ChaconTarazona | ||
from . import observables, utilities, datafiles | ||
from .version import __version__ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
'.version.version' imported but unused
pytim/__init__.py
Outdated
from .sasa import SASA | ||
from .willard_chandler import WillardChandler | ||
from .chacon_tarazona import ChaconTarazona | ||
from . import observables, utilities, datafiles |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
'.datafiles' imported but unused
'.observables' imported but unused
'.utilities' imported but unused
pytim/__init__.py
Outdated
from .gitim import GITIM | ||
from .sasa import SASA | ||
from .willard_chandler import WillardChandler | ||
from .chacon_tarazona import ChaconTarazona |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
'.chacon_tarazona.ChaconTarazona' imported but unused
pytim/__init__.py
Outdated
from .itim import ITIM | ||
from .gitim import GITIM | ||
from .sasa import SASA | ||
from .willard_chandler import WillardChandler |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
'.willard_chandler.WillardChandler' imported but unused
pytim/__init__.py
Outdated
from .simple_interface import SimpleInterface | ||
from .itim import ITIM | ||
from .gitim import GITIM | ||
from .sasa import SASA |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
'.sasa.SASA' imported but unused
pytim/__init__.py
Outdated
@@ -2,18 +2,18 @@ | |||
# vim: tabstop=4 expandtab shiftwidth=4 softtabstop=4 | |||
#from pytim.patches import patchTrajectory, patchOpenMM, patchMDTRAJ | |||
|
|||
from .simple_interface import SimpleInterface | |||
from .itim import ITIM | |||
from .gitim import GITIM |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
'.gitim.GITIM' imported but unused
pytim/__init__.py
Outdated
@@ -2,18 +2,18 @@ | |||
# vim: tabstop=4 expandtab shiftwidth=4 softtabstop=4 | |||
#from pytim.patches import patchTrajectory, patchOpenMM, patchMDTRAJ | |||
|
|||
from .simple_interface import SimpleInterface | |||
from .itim import ITIM |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
'.itim.ITIM' imported but unused
pytim/__init__.py
Outdated
@@ -2,18 +2,18 @@ | |||
# vim: tabstop=4 expandtab shiftwidth=4 softtabstop=4 | |||
#from pytim.patches import patchTrajectory, patchOpenMM, patchMDTRAJ | |||
|
|||
from .simple_interface import SimpleInterface |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
'.simple_interface.SimpleInterface' imported but unused
docs/source/conf.py
Outdated
@@ -16,6 +16,7 @@ | |||
# add these directories to sys.path here. If the directory is relative to the | |||
# documentation root, use os.path.abspath to make it absolute, like shown here. | |||
# | |||
import sphinx.ext.autodoc |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
'sphinx.ext.autodoc' imported but unused
Hi, I don't understand what you would like to merge. You should branch off the master branch, make your changes and then set up a pull request from your branch into the master branch here. I went through your branches, could it be what you want to merge is https://github.com/Zhu-Liu/pytim/tree/patch-1 ? If you have some trouble, you can have a look at https://opensource.com/article/19/7/create-pull-request-github or just contact me back. |
[cid:e00e058b-47b6-4ebb-b372-09964471878c]
Hi Dr. Sega,
I am really sorry that I forgot to answer your email.
I attached a code difference comparison, where the right side is the version directly downloaded from your GitHub and left is my slightly modified version.
Basically, there are two differences:
1. right Line 225 ---> left Line 323: I put a volume threshold value as the Gaussian density cutoff, which is cutting the Gaussian density around 90%.
For this part, you told me in the last emails that Pytim has already included this feature. Thus this may be necessary anymore.
2. right Line 231 ---> left Line 330: I comment out the vertices transfer. This will cause the off of the instantaneous interface location.
(At least in all my testing, for example, it will move the willard-chandler interface close to the central water phase.)
I also want to mention the x,z coordinates switching, which refers to right Line 213 and the function "generate_grid_in_box" from the "utilities_mesh" module.
I have no idea why the "generate_grid_in_box" function is written in that way (line 27--30, see below).
In my testing, this will result in the willard-chandler interface from the original xy-plane (along the z-axis) to the yz-plane (along the x-axis).
Best,
Zhu
[cid:83e51ac1-2faf-44f9-9e23-d7f4a2150cd8]
…--------------------------------------------------------------------------------
Dr. Zhu Liu
Postdoctoral Research Associate,
Department of Chemistry,
Washington State University
Tel. +1 509 592-1561
Email: [email protected]
https://aclark.chem.wsu.edu/
--------------------------------------------------------------------------------
________________________________
From: Marcello Sega <[email protected]>
Sent: Saturday, April 25, 2020 1:59 AM
To: Marcello-Sega/pytim <[email protected]>
Cc: Liu, Zhu <[email protected]>; Author <[email protected]>
Subject: Re: [Marcello-Sega/pytim] willard_chandler.py (#332)
Hi,
I don't understand what you would like to merge. You should branch off the master branch, make your changes and then set up a pull request from your branch into the master branch here.
I went through your branches, could it be what you want to merge is https://github.com/Zhu-Liu/pytim/tree/patch-1<https://urldefense.com/v3/__https://github.com/Zhu-Liu/pytim/tree/patch-1__;!!JmPEgBY0HMszNaDT!4fMwHJS7AM9VGvjgqrs0SoBhXNEbuEf90c3vEC017SpnfWEjs3EQF8k-BdCPi3E$> ?
If you have some trouble, you can have a look at https://opensource.com/article/19/7/create-pull-request-github<https://urldefense.com/v3/__https://opensource.com/article/19/7/create-pull-request-github__;!!JmPEgBY0HMszNaDT!4fMwHJS7AM9VGvjgqrs0SoBhXNEbuEf90c3vEC017SpnfWEjs3EQF8k-8lRatns$> or just contact me back.
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub<https://urldefense.com/v3/__https://github.com/Marcello-Sega/pytim/pull/332*issuecomment-619346518__;Iw!!JmPEgBY0HMszNaDT!4fMwHJS7AM9VGvjgqrs0SoBhXNEbuEf90c3vEC017SpnfWEjs3EQF8k-ekCA3GM$>, or unsubscribe<https://urldefense.com/v3/__https://github.com/notifications/unsubscribe-auth/AE5OTV4E3KQBPWKGPQ3NLQ3ROKQ7ZANCNFSM4MHIDOBQ__;!!JmPEgBY0HMszNaDT!4fMwHJS7AM9VGvjgqrs0SoBhXNEbuEf90c3vEC017SpnfWEjs3EQF8k-Qvo0JsM$>.
|
pytim/version.py
Outdated
@@ -1 +1 @@ | |||
__version__ = '0.8.1' | |||
__version__ = '0.8.2' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Black would make changes.
pytim/interface.py
Outdated
label_max = np.argmax(counts) | ||
ids_max = np.where(labels == label_max)[0] | ||
self.cluster_group = self.cluster_group[ids_max] | ||
except AttributeError: # biggest_cluster_only not set |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
at least two spaces before inline comment
pytim/interface.py
Outdated
<AtomGroup with 2025 atoms> | ||
|
||
|
||
>>> inter = pytim.SASA( g, alpha=2.5, max_layers=2, cluster_cut=3.5, biggest_cluster_only=True, molecular=True) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
line too long (119 > 79 characters)
pytim/interface.py
Outdated
>>> u = mda.Universe(ANTAGONISTIC_GRO) | ||
>>> g = u.atoms.select_atoms('resname bph4') | ||
>>> # Define the interface | ||
>>> inter = pytim.SASA( g, alpha=2.5, max_layers=2, cluster_cut=3.5, biggest_cluster_only=False, molecular=True) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
line too long (120 > 79 characters)
pytim/datafiles/__init__.py
Outdated
@@ -293,6 +292,9 @@ def _vdwradii_gmx(self, filename): | |||
ILBENZENE_GRO = resource_filename('pytim', 'data/ilbenzene.gro') | |||
pytim_data.add('ILBENZENE_GRO', 'conf', 'GRO', 'BMIM PF4 / benzene interface') | |||
|
|||
ANTAGONISTIC_GRO = resource_filename('pytim', 'data/antagonistic.gro') | |||
pytim_data.add('ANTAGONISTIC_GRO', 'conf', 'GRO', '3-Methylpyridine, Sodium Tetraphenylborate and water') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
line too long (105 > 79 characters)
pytim/observables/orientation.py
Outdated
normal_component[normal_component<-1.0]=-1.0 | ||
orthogonal_component = np.sqrt(1-normal_component**2) | ||
phi = np.mod(np.arctan2(normal_component, orthogonal_component),np.pi/2) | ||
return costheta,phi |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
missing whitespace after ','
pytim/observables/orientation.py
Outdated
normal_component[normal_component>1.0]=1.0 | ||
normal_component[normal_component<-1.0]=-1.0 | ||
orthogonal_component = np.sqrt(1-normal_component**2) | ||
phi = np.mod(np.arctan2(normal_component, orthogonal_component),np.pi/2) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
line too long (80 > 79 characters)
missing whitespace after ','
pytim/observables/orientation.py
Outdated
normal_component = np.sum(v*v_normal,axis=1) | ||
# avoid numerical errors | ||
normal_component[normal_component>1.0]=1.0 | ||
normal_component[normal_component<-1.0]=-1.0 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
missing whitespace around operator
pytim/observables/orientation.py
Outdated
v = v / np.linalg.norm(v,axis=1)[:,np.newaxis] | ||
normal_component = np.sum(v*v_normal,axis=1) | ||
# avoid numerical errors | ||
normal_component[normal_component>1.0]=1.0 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
missing whitespace around operator
pytim/observables/orientation.py
Outdated
# the surface normal projected on the plane with normal v_dipole | ||
v = (v_surface-v_dipole*costheta[:,np.newaxis]) | ||
v = v / np.linalg.norm(v,axis=1)[:,np.newaxis] | ||
normal_component = np.sum(v*v_normal,axis=1) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
missing whitespace after ','
pytim/observables/orientation.py
Outdated
options=[''] | ||
if molecular: options=['molecular'] | ||
self.molecular_plane_vector = Orientation(universe,options=options+['normal']) | ||
self.dipole_vector = Orientation(universe,options=options) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
missing whitespace after ','
pytim/observables/orientation.py
Outdated
def __init__(self, universe, molecular=False): | ||
options=[''] | ||
if molecular: options=['molecular'] | ||
self.molecular_plane_vector = Orientation(universe,options=options+['normal']) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
line too long (87 > 79 characters)
missing whitespace after ','
multiple spaces before operator
pytim/observables/orientation.py
Outdated
|
||
def __init__(self, universe, molecular=False): | ||
options=[''] | ||
if molecular: options=['molecular'] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
missing whitespace around operator
multiple statements on one line (colon)
pytim/observables/orientation.py
Outdated
""" | ||
|
||
def __init__(self, universe, molecular=False): | ||
options=[''] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
missing whitespace around operator
pytim/observables/orientation.py
Outdated
The angles are computed according to Jedlovszky et al., Phys. Chem. Chem. Phys., 2004, 6, 1874–1879 | ||
|
||
:param: bool molecular: if True, uses the option "molecular=True" in the calculation of the | ||
molecular axes (using pytim.observables.Orientation) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
line too long (85 > 79 characters)
pytim/observables/orientation.py
Outdated
|
||
The angles are computed according to Jedlovszky et al., Phys. Chem. Chem. Phys., 2004, 6, 1874–1879 | ||
|
||
:param: bool molecular: if True, uses the option "molecular=True" in the calculation of the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
line too long (100 > 79 characters)
pytim/observables/orientation.py
Outdated
""" Two angles characterizing the orientation next to a surface of an axisymmetric molecule | ||
like water. | ||
|
||
The angles are computed according to Jedlovszky et al., Phys. Chem. Chem. Phys., 2004, 6, 1874–1879 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
line too long (107 > 79 characters)
pytim/observables/orientation.py
Outdated
@@ -57,3 +57,80 @@ def compute(self, inp, kargs=None): | |||
v = np.array(a + b) | |||
v = np.array([el / np.sqrt(np.dot(el, el)) for el in v]) | |||
return v | |||
|
|||
class BivariateAngles(Observable): | |||
""" Two angles characterizing the orientation next to a surface of an axisymmetric molecule |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
line too long (91 > 79 characters)
pytim/observables/orientation.py
Outdated
@@ -57,3 +57,80 @@ def compute(self, inp, kargs=None): | |||
v = np.array(a + b) | |||
v = np.array([el / np.sqrt(np.dot(el, el)) for el in v]) | |||
return v | |||
|
|||
class BivariateAngles(Observable): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
expected 2 blank lines, found 1
pytim/observables/orientation.py
Outdated
@@ -28,7 +28,7 @@ def __init__(self, universe, options=''): | |||
def compute(self, inp, kargs=None): | |||
"""Compute the observable. | |||
|
|||
:param ndarray inp: the input atom group. The length be a multiple | |||
:param ndarray inp: the input atom group. The length must be a multiple |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
line too long (80 > 79 characters)
pytim/observables/__init__.py
Outdated
@@ -19,5 +19,5 @@ | |||
from .rdf2d import RDF2D | |||
from .free_volume import FreeVolume | |||
from .correlator import Correlator | |||
from .orientation import Orientation | |||
from .orientation import Orientation,BivariateAngles |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
'.orientation.BivariateAngles' imported but unused
'.orientation.Orientation' imported but unused
Black would make changes.
missing whitespace after ','
pytim/willard_chandler.py
Outdated
@@ -226,6 +226,7 @@ def _assign_layers(self): | |||
volume, None, spacing=tuple(spacing)) | |||
# note that len(normals) == len(verts): they are normals | |||
# at the vertices, and not normals of the faces | |||
self.triangulated_surface = [verts, faces, normals] | |||
# verts and normals have x and z flipped because skimage uses zyx ordering | |||
self.triangulated_surface = [np.fliplr(verts), faces, np.fliplr(normals)] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
line too long (81 > 79 characters)
pytim/willard_chandler.py
Outdated
@@ -226,6 +226,7 @@ def _assign_layers(self): | |||
volume, None, spacing=tuple(spacing)) | |||
# note that len(normals) == len(verts): they are normals | |||
# at the vertices, and not normals of the faces | |||
self.triangulated_surface = [verts, faces, normals] | |||
# verts and normals have x and z flipped because skimage uses zyx ordering |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
line too long (82 > 79 characters)
pytim/itim.py
Outdated
@@ -195,7 +195,7 @@ def __init__(self, | |||
**kargs): | |||
|
|||
self.autoassign = autoassign | |||
|
|||
self.biggest_cluster_only = True # necessary for ITIM |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
at least two spaces before inline comment
pytim/interface.py
Outdated
label_max = np.argmax(counts) | ||
ids_max = np.where(labels == label_max)[0] | ||
self.cluster_group = self.cluster_group[ids_max] | ||
else: # we still filter out molecules which do not belong to any cluster |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
at least two spaces before inline comment
line too long (84 > 79 characters)
pytim/itim.py
Outdated
else: | ||
for index, group in enumerate(self._assign_one_side(up, sort[::-1], _x, _y, _z, _radius)): | ||
self._layers[up][index] = group | ||
for index, group in enumerate(self._assign_one_side(low, sort[::], _x, _y, _z, _radius)): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
line too long (101 > 79 characters)
pytim/itim.py
Outdated
for q in queue: | ||
q.close() | ||
else: | ||
for index, group in enumerate(self._assign_one_side(up, sort[::-1], _x, _y, _z, _radius)): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
line too long (102 > 79 characters)
pytim/itim.py
Outdated
# info about universe is lost (do not know why yet) | ||
# must use self._layers[uplow][index] = | ||
# self.universe.atoms[group.indices] | ||
self._layers[uplow][index] = self.universe.atoms[group.indices] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
line too long (83 > 79 characters)
b90f41b
to
49c6764
Compare
From Line 210: