Skip to content

Commit

Permalink
Remove TODO comments (#876)
Browse files Browse the repository at this point in the history
* remove todo, see issue #875

* see #877

* see #878

* see #879

* see #880

* see #881

* see #882

* see #883

* see #884

* see #885

* see #886
  • Loading branch information
jennyfothergill authored Apr 27, 2021
1 parent d18d736 commit 1096d19
Show file tree
Hide file tree
Showing 10 changed files with 24 additions and 50 deletions.
1 change: 0 additions & 1 deletion mosdef_gomc/coarse_graining.py
Original file line number Diff line number Diff line change
Expand Up @@ -180,7 +180,6 @@ def _create_proxy_labels(real_thing, memo):
if not is_leaf(real_thing):
for label, part in real_thing.labels.items():
if isinstance(part, list):
# TODO support lists with labels
continue
if part in memo:
memo[real_thing].labels[label] = memo[part]
Expand Down
5 changes: 2 additions & 3 deletions mosdef_gomc/compound.py
Original file line number Diff line number Diff line change
Expand Up @@ -302,7 +302,7 @@ def particles_by_element(self, element):
Parameters
----------
name : str or ele.Element
element abbreviation or element
element abbreviation or element
Yields
------
Expand Down Expand Up @@ -983,7 +983,6 @@ def box(self, box):
raise TypeError("box must be specified as an mbuild.Box")
if self.port_particle and box is not None:
raise ValueError("Ports cannot have a box")
# TODO: Fix this for non-orthogonal boxes
# Make sure the box is bigger than the bounding box
if box is not None:
if (box.lengths < self.boundingbox.lengths).any():
Expand All @@ -997,7 +996,7 @@ def box(self, box):
@property
def element(self):
return self._element

@element.setter
def element(self, element):
if element is None:
Expand Down
3 changes: 0 additions & 3 deletions mosdef_gomc/conversion.py
Original file line number Diff line number Diff line change
Expand Up @@ -768,9 +768,6 @@ def from_pybel(pybel_mol,
else:
if not ignore_box_warn:
warn("No unitcell detected for pybel.Molecule {}".format(pybel_mol))
# TODO: Decide how to gather PBC information from openbabel. Options may
# include storing it in .periodicity or writing a separate function
# that returns the box.
return compound


Expand Down
6 changes: 0 additions & 6 deletions mosdef_gomc/coordinate_transform.py
Original file line number Diff line number Diff line change
Expand Up @@ -326,7 +326,6 @@ def _create_equivalence_transform(equiv):
"and pair[1] is a {1}".format(type(pair[0]), type(pair[1]))
)

# TODO: vstack is slow, replace with list concatenation
if not pair[0].children:
self_points = np.vstack([self_points, pair[0].pos])
other_points = np.vstack([other_points, pair[1].pos])
Expand Down Expand Up @@ -381,7 +380,6 @@ def equivalence_transform(compound, from_positions, to_positions, add_bond=True)
if add_bond:
if isinstance(from_positions, Port) and isinstance(to_positions, Port):
if not from_positions.anchor or not to_positions.anchor:
# TODO: I think warnings is undefined here
warn("Attempting to form bond from port that has no anchor")
else:
from_positions.anchor.parent.add_bond(
Expand All @@ -400,10 +398,6 @@ def _choose_correct_port(from_port, to_port):
chooses the set of 4 atoms that makes the anchor atoms not overlap which is
the intended behavior for most use-cases.
TODO: -Increase robustness for cases where the anchors are a different
distance from their respective ports.
-Provide options in `force_overlap` to override this behavior.
Parameters
----------
from_port : mb.Port
Expand Down
2 changes: 0 additions & 2 deletions mosdef_gomc/formats/compound.proto
Original file line number Diff line number Diff line change
Expand Up @@ -29,8 +29,6 @@ message Compound {
int64 id = 6; // this is simply a unique number for this compound
repeated Vec2 bonds = 7; // a list of 2-tuples
Element element = 8;

// Todo: Maybe need something for port particles
}


25 changes: 12 additions & 13 deletions mosdef_gomc/formats/lammpsdata.py
Original file line number Diff line number Diff line change
Expand Up @@ -24,16 +24,16 @@ def _check_minsmaxs(mins, maxs):
else:
return False

def write_lammpsdata(structure, filename, atom_style='full',
def write_lammpsdata(structure, filename, atom_style='full',
unit_style='real',
mins=None,
maxs=None,
detect_forcefield_style=True, nbfix_in_data_file=True,
use_urey_bradleys=False,
use_rb_torsions=True, use_dihedrals=False):
"""Output a LAMMPS data file.
Outputs a LAMMPS data file in the 'full' atom style format. Default units are
Outputs a LAMMPS data file in the 'full' atom style format. Default units are
'real' units. See http://lammps.sandia.gov/doc/atom_style.html for
more information on atom styles.
Expand All @@ -58,7 +58,7 @@ def write_lammpsdata(structure, filename, atom_style='full',
maxs : list
maximum box dimension in x, y, z directions
detect_forcefield_style: boolean
If True, format lammpsdata parameters based on the contents of
If True, format lammpsdata parameters based on the contents of
the parmed Structure
use_urey_bradleys: boolean
If True, will treat angles as CHARMM-style angles with urey bradley terms
Expand All @@ -67,7 +67,7 @@ def write_lammpsdata(structure, filename, atom_style='full',
If True, will treat dihedrals OPLS-style torsions while looking for
`structure.rb_torsions`
use_dihedrals:
If True, will treat dihedrals as CHARMM-style dihedrals while looking for
If True, will treat dihedrals as CHARMM-style dihedrals while looking for
`structure.dihedrals`
Notes
Expand All @@ -89,7 +89,7 @@ def write_lammpsdata(structure, filename, atom_style='full',
# Check if structure is paramterized
if unit_style == 'lj':
if any([atom.sigma for atom in structure.atoms]) is None:
raise ValueError('LJ units specified but one or more atoms has undefined LJ parameters.')
raise ValueError('LJ units specified but one or more atoms has undefined LJ parameters.')

xyz = np.array([[atom.xx,atom.xy,atom.xz] for atom in structure.atoms])

Expand Down Expand Up @@ -149,8 +149,7 @@ def write_lammpsdata(structure, filename, atom_style='full',
xyz = xyz / sigma_conversion_factor
charges = (charges*1.6021e-19) / np.sqrt(4*np.pi*(sigma_conversion_factor*1e-10)*
(epsilon_conversion_factor*4184)*epsilon_0)
charges[np.isinf(charges)] = 0
# TODO: FIX CHARGE UNIT CONVERSION
charges[np.isinf(charges)] = 0
else:
sigma_conversion_factor = 1
epsilon_conversion_factor = 1
Expand All @@ -170,7 +169,7 @@ def write_lammpsdata(structure, filename, atom_style='full',
warn('Explicit box bounds (i.e., mins and maxs) were not provided. Box bounds are assumed to be min = 0 and max = length in each direction. This may not produce a system with the expected spatial location and may cause non-periodic systems to fail. Bounds can be defined explicitly by passing the them to the write_lammpsdata function or by passing box info to the save function.')
# Divide by conversion factor
box.maxs /= sigma_conversion_factor

# Lammps syntax depends on the functional form
# Infer functional form based on the properties of the structure
if detect_forcefield_style:
Expand Down Expand Up @@ -242,11 +241,11 @@ def write_lammpsdata(structure, filename, atom_style='full',
dihedral_types, unique_dihedral_types = _get_dihedral_types(
structure, use_rb_torsions, use_dihedrals,
epsilon_conversion_factor)

if impropers:
improper_types, unique_improper_types = _get_impropers(structure,
epsilon_conversion_factor)


with open(filename, 'w') as data:
data.write(filename+' - created by mBuild; units = {}\n\n'.format(
Expand Down Expand Up @@ -324,7 +323,7 @@ def write_lammpsdata(structure, filename, atom_style='full',
epsilon_dict = dict([(unique_types.index(atom_type)+1,epsilon) for atom_type,epsilon in zip(types,epsilons)])
sigma_dict = dict([(unique_types.index(atom_type)+1,sigma) for atom_type,sigma in zip(types,sigmas)])
forcefield_dict = dict([(unique_types.index(atom_type)+1,forcefield) for atom_type,forcefield in zip(types,forcefields)])



# Modified cross-interactions
Expand Down Expand Up @@ -543,7 +542,7 @@ def _get_angle_types(structure, use_urey_bradleys,
ub_k = ub.type.k
ub_req = ub.type.req
charmm_angle_types.append((round(angle.type.k*(
sigma_conversion_factor**2/epsilon_conversion_factor),3),
sigma_conversion_factor**2/epsilon_conversion_factor),3),
round(angle.type.theteq,3),
round(ub_k/epsilon_conversion_factor, 3),
round(ub_req, 3),
Expand Down
24 changes: 9 additions & 15 deletions mosdef_gomc/formats/protobuf.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,14 +12,9 @@ def write_pb2(cmpd, filename, binary=True):
---------
cmpd : mb.Compound
filename : str
binary: bool, default True
binary: bool, default True
If True, will print a binary file
If False, will print to a text file
Todo: This could be more elegantly detected
Notes
----
Todo: Handle Ports in the protocol buffer (.proto) and in this writer/reader
"""
cmpd_to_proto = {}

Expand All @@ -29,7 +24,7 @@ def write_pb2(cmpd, filename, binary=True):

for sub_cmpd in cmpd.successors():
parent_cmpd = sub_cmpd.parent
sub_proto = cmpd_to_proto[parent_cmpd].children.add()
sub_proto = cmpd_to_proto[parent_cmpd].children.add()
sub_proto = _mb_to_proto(sub_cmpd, sub_proto)
cmpd_to_proto[sub_cmpd] = sub_proto

Expand All @@ -49,10 +44,9 @@ def read_pb2(filename, binary=True):
Parameters
---------
filename : str
binary: bool, default True
binary: bool, default True
If True, will print a binary file
If False, will print to a text file
Todo: This could be more elegantly detected
Returns
------
Expand Down Expand Up @@ -99,7 +93,7 @@ def _mb_to_proto(cmpd, proto):
proto.element.symbol = cmpd.element.symbol
proto.element.atomic_number = cmpd.element.atomic_number
proto.element.mass = cmpd.element.mass

return proto

def _add_proto_bonds(cmpd, proto):
Expand All @@ -126,7 +120,7 @@ def _proto_successors(proto):
Notes
-----
Base Case: there are no children to the proto, just return
Base Case: there are no children to the proto, just return
Recursion: First look at proto's children and return these children (sub_proto)
Then make the recursive call to look at all the sub_proto's successors
This is similar to mb.Compound().successors()
Expand All @@ -140,12 +134,12 @@ def _proto_successors(proto):
yield (sub_sub_proto, parent_proto)

def _proto_to_mb(proto):
""" Given compound_pb2.Compound, create mb.Compound
""" Given compound_pb2.Compound, create mb.Compound
Parameters
----------
proto: compound_pb2.Compound()
"""
if proto.element.symbol is '':
elem = None
Expand All @@ -154,7 +148,7 @@ def _proto_to_mb(proto):
return mb.Compound(name=proto.name,
pos=[proto.pos.x, proto.pos.y, proto.pos.z],
charge=proto.charge,
periodicity=[proto.periodicity.x, proto.periodicity.y,
periodicity=[proto.periodicity.x, proto.periodicity.y,
proto.periodicity.z],
element=elem)

Expand Down
1 change: 0 additions & 1 deletion mosdef_gomc/lib/surfaces/amorphous_silica_surface.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ def __init__(self, surface_roughness=1.0):
super(AmorphousSilicaSurface, self).__init__()

if surface_roughness == 1.0:
# TODO: description of how this surface was generated/citation
mb.load('amorphous_silica_sr1.0.pdb', compound=self,
relative_to_module=self.__module__)
self.periodicity = np.array([5.4366, 4.7082, 0.0])
Expand Down
2 changes: 1 addition & 1 deletion mosdef_gomc/pattern.py
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ def __init__(self, points, orientations=None, scale=None, **kwargs):
self.points = points
if orientations is None:
orientations = dict()
self.orientations = orientations # TODO: implement for more patterns
self.orientations = orientations
if scale is not None:
self.scale(scale)

Expand Down
5 changes: 0 additions & 5 deletions mosdef_gomc/tests/test_compound.py
Original file line number Diff line number Diff line change
Expand Up @@ -1162,17 +1162,12 @@ def test_from_pybel_molecule(self, extension):
pybel = import_('pybel')
chol = list(pybel.readfile(extension,
get_fn('cholesterol.{}'.format(extension))))[0]
# TODO: Actually store the box information
cmpd = mb.Compound()
cmpd.from_pybel(chol)
assert chol.OBMol.NumAtoms() == cmpd.n_particles
assert chol.OBMol.NumBonds() == cmpd.n_bonds
first_atom = chol.OBMol.GetAtom(1)
assert np.allclose(cmpd[0].pos, [first_atom.GetX()/10, first_atom.GetY()/10, first_atom.GetZ()/10])
#assert np.allclose(box.lengths,
# [chol.unitcell.GetA()/10, chol.unitcell.GetB()/10,
# chol.unitcell.GetC()/10],
# rtol=1e-3)

@pytest.mark.parametrize('test_smiles', [
"CCO",
Expand Down

0 comments on commit 1096d19

Please sign in to comment.