Skip to content

Commit bf82052

Browse files
RainierBarrettpre-commit-ci[bot]jaclark5daico007
authored
Issue 1080 (#1088)
* Update fill_region bounds and docstring to be more explicit * Fix type error caused by treating box as list * fix backword none check * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * [pre-commit.ci] pre-commit autoupdate (#1089) * [pre-commit.ci] pre-commit autoupdate updates: - [github.com/psf/black: 22.12.0 → 23.1.0](psf/black@22.12.0...23.1.0) * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --------- Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> * Lammps lj unit charges (#1086) * Produce n=2 when ports are not capped * Add option to not scale charges in lj units, and update docs for consistency * Update mbuild/formats/lammpsdata.py Co-authored-by: Co Quach <43968221+daico007@users.noreply.github.com> --------- Co-authored-by: Co Quach <43968221+daico007@users.noreply.github.com> * Update fill_region bounds and docstring to be more explicit * Fix type error caused by treating box as list * fix backword none check * Add unit tests for new errors * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --------- Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> Co-authored-by: Jennifer A Clark <jennifer.a.clark13@gmail.com> Co-authored-by: Co Quach <43968221+daico007@users.noreply.github.com>
1 parent a6aa3a6 commit bf82052

File tree

2 files changed

+51
-3
lines changed

2 files changed

+51
-3
lines changed

mbuild/packing.py

Lines changed: 24 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -316,9 +316,13 @@ def fill_region(
316316
fix_orientation : bool or list of bools
317317
Specify that compounds should not be rotated when filling the box,
318318
default=False.
319-
bounds : list-like of floats [minx, miny, minz, maxx, maxy, maxz], units nm, default=None
320-
Bounding within box to pack compounds, if you want to pack within a bounding
319+
bounds : list-like of list-likes of floats [[min_x, min_y, min_z, max_x, max_y, max_z], ...], units nm, default=None
320+
Bounding(s) within box to pack compound(s). To pack within a bounding
321321
area that is not the full extent of the region, bounds are required.
322+
Each item of `compound` must have its own bound specified. Use `None`
323+
to indicate a given compound is not bounded, e.g.
324+
`[ [0., 0., 1., 2., 2., 2.], None]` to bound only the first element
325+
of `compound` and not the second.
322326
temp_file : str, default=None
323327
File name to write PACKMOL raw output to.
324328
update_port_locations : bool, default=False
@@ -354,6 +358,23 @@ def fill_region(
354358
"`compound`, `n_compounds`, and `fix_orientation` must be of "
355359
"equal length."
356360
)
361+
if bounds is not None:
362+
if not isinstance(bounds, (list)):
363+
raise TypeError(
364+
"`bounds` must be a list of one or more bounding boxes "
365+
"and/or `None` for each item in `compound`."
366+
)
367+
if len(bounds) != len(n_compounds):
368+
raise ValueError(
369+
"`compound` and `bounds` must be of equal length. Use `None` "
370+
"for non-bounded items in `compound`."
371+
)
372+
for bound in bounds:
373+
if not isinstance(bound, (Box, list)):
374+
raise ValueError(
375+
"Each bound in `bounds` must be `None`, `Box`, or a "
376+
"list of [min_x, min_y, min_z, max_x, max_y, max_z]."
377+
)
357378

358379
# See if region is a single region or list
359380
my_regions = []
@@ -373,7 +394,7 @@ def fill_region(
373394
f"expected a list of type: list or mbuild.Box, was provided {region} of type: {type(region)}"
374395
)
375396
container = []
376-
if not bounds:
397+
if bounds is None:
377398
bounds = []
378399
for bound, reg in zip_longest(bounds, my_regions, fillvalue=None):
379400
if bound is None:

mbuild/tests/test_packing.py

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -138,6 +138,33 @@ def test_fill_region_incorrect_type(self, ethane):
138138
compound=[ethane], n_compounds=[2], region=box1, bounds=None
139139
)
140140

141+
def test_fill_region_bounds_not_list(self, ethane):
142+
box1 = Box(lengths=[2, 2, 2], angles=[90.0, 90.0, 90.0])
143+
with pytest.raises(TypeError):
144+
mb.fill_region(
145+
compound=[ethane], n_compounds=[2], region=box1, bounds=box1
146+
)
147+
148+
def test_fill_region_incorrect_bounds_amount(self, ethane, h2o):
149+
box1 = Box(lengths=[2, 2, 2], angles=[90.0, 90.0, 90.0])
150+
with pytest.raises(ValueError):
151+
mb.fill_region(
152+
compound=[ethane, h2o],
153+
n_compounds=[2, 2],
154+
region=box1,
155+
bounds=[box1],
156+
)
157+
158+
def test_fill_region_incorrect_bounds_types(self, ethane, h2o):
159+
box1 = Box(lengths=[2, 2, 2], angles=[90.0, 90.0, 90.0])
160+
with pytest.raises(ValueError):
161+
mb.fill_region(
162+
compound=[ethane, h2o],
163+
n_compounds=[2, 2],
164+
region=box1,
165+
bounds=[1.0, 1.0, 1.0],
166+
)
167+
141168
def test_box_no_bound(self, ethane):
142169
box1 = Box(lengths=[2, 2, 2], angles=[90.0, 90.0, 90.0])
143170
mb.fill_region(

0 commit comments

Comments
 (0)