From 3d3a27f841b03c765196a84c3a5dfd0e9b76cc70 Mon Sep 17 00:00:00 2001 From: Ethan Peterson Date: Mon, 24 Feb 2020 11:55:42 -0500 Subject: [PATCH] applied suggestions from code review --- docs/source/usersguide/install.rst | 2 +- openmc/model/funcs.py | 47 +----------------- openmc/surface.py | 79 +++++++++++++++++------------- setup.py | 3 +- tests/unit_tests/test_surface.py | 27 +++++----- 5 files changed, 63 insertions(+), 95 deletions(-) diff --git a/docs/source/usersguide/install.rst b/docs/source/usersguide/install.rst index 66440a3c3..ec10fea4f 100644 --- a/docs/source/usersguide/install.rst +++ b/docs/source/usersguide/install.rst @@ -404,7 +404,7 @@ to install the Python package in :ref:`"editable" mode `. Prerequisites ------------- -The Python API works with Python 3.4+. In addition to Python itself, the API +The Python API works with Python 3.5+. In addition to Python itself, the API relies on a number of third-party packages. All prerequisites can be installed using Conda_ (recommended), pip_, or through the package manager in most Linux distributions. To run simulations in parallel using MPI, it is recommended to diff --git a/openmc/model/funcs.py b/openmc/model/funcs.py index 3ee505240..9999c383f 100644 --- a/openmc/model/funcs.py +++ b/openmc/model/funcs.py @@ -373,52 +373,7 @@ def get_hexagonal_prism(*args, **kwargs): return hexagonal_prism(*args, **kwargs) -def cylinder_from_points(p1, p2, r, **kwargs): - """Return cylinder defined by two points passing through its center. - - Parameters - ---------- - p1, p2 : 3-tuples - Coordinates of two points that pass through the center of the cylinder - r : float - Radius of the cylinder - kwargs : dict - Keyword arguments passed to the :class:`openmc.Quadric` constructor - - Returns - ------- - openmc.Quadric - Quadric surface representing the cylinder. - - """ - # Get x, y, z coordinates of two points - x1, y1, z1 = p1 - x2, y2, z2 = p2 - - # Define intermediate terms - dx = x2 - x1 - dy = y2 - y1 - dz = z2 - z1 - cx = y1*z2 - y2*z1 - cy = x2*z1 - x1*z2 - cz = x1*y2 - x2*y1 - - # Given p=(x,y,z), p1=(x1, y1, z1), p2=(x2, y2, z2), the equation for the - # cylinder can be derived as r = |(p - p1) тип (p - p2)| / |p2 - p1|. - # Expanding out all terms and grouping according to what Quadric expects - # gives the following coefficients. - kwargs['a'] = dy*dy + dz*dz - kwargs['b'] = dx*dx + dz*dz - kwargs['c'] = dx*dx + dy*dy - kwargs['d'] = -2*dx*dy - kwargs['e'] = -2*dy*dz - kwargs['f'] = -2*dx*dz - kwargs['g'] = 2*(cy*dz - cz*dy) - kwargs['h'] = 2*(cz*dx - cx*dz) - kwargs['j'] = 2*(cx*dy - cy*dx) - kwargs['k'] = cx*cx + cy*cy + cz*cz - (dx*dx + dy*dy + dz*dz)*r*r - - return Quadric(**kwargs) +cylinder_from_points = Cylinder.from_points def subdivide(surfaces): diff --git a/openmc/surface.py b/openmc/surface.py index a1696b08c..4d0434a4b 100644 --- a/openmc/surface.py +++ b/openmc/surface.py @@ -3,13 +3,14 @@ from collections import OrderedDict from copy import deepcopy from numbers import Real from xml.etree import ElementTree as ET -from warnings import warn +from warnings import warn, catch_warnings, simplefilter +import math import numpy as np from openmc.checkvalue import check_type, check_value from openmc.region import Region, Intersection, Union -from openmc.mixin import IDManagerMixin +from openmc.mixin import IDManagerMixin, IDWarning _BOUNDARY_TYPES = ['transmission', 'vacuum', 'reflective', 'periodic', 'white'] @@ -369,10 +370,32 @@ class PlaneMixin(metaclass=ABCMeta): return np.array((a, b, c)) / math.sqrt(a*a + b*b + c*c) def bounding_box(self, side): + """Determine an axis-aligned bounding box. + + An axis-aligned bounding box for Plane half-spaces is represented by + its lower-left and upper-right coordinates. If the half-space is + unbounded in a particular direction, numpy.inf is used to represent + infinity. + + Parameters + ---------- + side : {'+', '-'} + Indicates the negative or positive half-space + + Returns + ------- + numpy.ndarray + Lower-left coordinates of the axis-aligned bounding box for the + desired half-space + numpy.ndarray + Upper-right coordinates of the axis-aligned bounding box for the + desired half-space + + """ # Compute the bounding box based on the normal vector to the plane nhat = self._get_normal() - lb = np.array([-np.inf, -np.inf, -np.inf]) - ub = np.array([np.inf, np.inf, np.inf]) + ll = np.array([-np.inf, -np.inf, -np.inf]) + ur = np.array([np.inf, np.inf, np.inf]) # If the plane is axis aligned, find the proper bounding box if np.any(np.isclose(np.abs(nhat), 1., rtol=0., atol=self._atol)): sign = nhat.sum() @@ -380,16 +403,16 @@ class PlaneMixin(metaclass=ABCMeta): vals = [d/val if round(val) != 0 else np.nan for val in (a, b, c)] if side == '-': if sign > 0: - ub = np.array([v if not np.isnan(v) else np.inf for v in vals]) + ur = np.array([v if not np.isnan(v) else np.inf for v in vals]) else: - lb = np.array([v if ~np.isnan(v) else -np.inf for v in vals]) + ll = np.array([v if not np.isnan(v) else -np.inf for v in vals]) elif side == '+': if sign > 0: - lb = np.array([v if ~np.isnan(v) else -np.inf for v in vals]) + ll = np.array([v if not np.isnan(v) else -np.inf for v in vals]) else: - ub = np.array([v if ~np.isnan(v) else np.inf for v in vals]) + ur = np.array([v if not np.isnan(v) else np.inf for v in vals]) - return (lb, ub) + return (ll, ur) def evaluate(self, point): """Evaluate the surface equation at a given point. @@ -1180,18 +1203,12 @@ class Cylinder(QuadricMixin, Surface): """ # This method overrides Surface.to_xml_element to generate a Quadric # since the C++ layer doesn't support Cylinders right now - element = ET.Element("surface") - element.set("id", str(self._id)) - - if len(self._name) > 0: - element.set("name", str(self._name)) - - element.set("type", 'quadric') - if self.boundary_type != 'transmission': - element.set("boundary", self.boundary_type) - element.set("coeffs", ' '.join([str(c) for c in self._get_base_coeffs()])) - - return element + with catch_warnings(): + simplefilter('ignore', IDWarning) + kwargs = {'boundary_type': self.boundary_type, 'name': self.name, + 'surface_id': self.id} + quad_rep = Quadric(*self._get_base_coeffs(), **kwargs) + return quad_rep.to_xml_element() class XCylinder(QuadricMixin, Surface): @@ -1881,19 +1898,13 @@ class Cone(QuadricMixin, Surface): """ # This method overrides Surface.to_xml_element to generate a Quadric - # since the C++ layer doesn't support Cylinders right now - element = ET.Element("surface") - element.set("id", str(self._id)) - - if len(self._name) > 0: - element.set("name", str(self._name)) - - element.set("type", 'quadric') - if self.boundary_type != 'transmission': - element.set("boundary", self.boundary_type) - element.set("coeffs", ' '.join([str(c) for c in self._get_base_coeffs()])) - - return element + # since the C++ layer doesn't support Cones right now + with catch_warnings(): + simplefilter('ignore', IDWarning) + kwargs = {'boundary_type': self.boundary_type, 'name': self.name, + 'surface_id': self.id} + quad_rep = Quadric(*self._get_base_coeffs(), **kwargs) + return quad_rep.to_xml_element() class XCone(QuadricMixin, Surface): diff --git a/setup.py b/setup.py index 99db83148..c193645c3 100755 --- a/setup.py +++ b/setup.py @@ -56,14 +56,13 @@ kwargs = { 'Topic :: Scientific/Engineering' 'Programming Language :: C++', 'Programming Language :: Python :: 3', - 'Programming Language :: Python :: 3.4', 'Programming Language :: Python :: 3.5', 'Programming Language :: Python :: 3.6', 'Programming Language :: Python :: 3.7', ], # Dependencies - 'python_requires': '>=3.4', + 'python_requires': '>=3.5', 'install_requires': [ 'numpy>=1.9', 'h5py', 'scipy', 'ipython', 'matplotlib', 'pandas', 'lxml', 'uncertainties' diff --git a/tests/unit_tests/test_surface.py b/tests/unit_tests/test_surface.py index 703010a05..86e01b6af 100644 --- a/tests/unit_tests/test_surface.py +++ b/tests/unit_tests/test_surface.py @@ -370,23 +370,26 @@ def test_cylinder_from_points_axis(): # (x - 3)^2 + (y - 4)^2 = 2^2 # x^2 + y^2 - 6x - 8y + 21 = 0 s = openmc.model.cylinder_from_points((3., 4., 0.), (3., 4., 1.), 2.) - assert (s.a, s.b, s.c) == pytest.approx((1., 1., 0.)) - assert (s.d, s.e, s.f) == pytest.approx((0., 0., 0.)) - assert (s.g, s.h, s.j) == pytest.approx((-6., -8., 0.)) - assert s.k == pytest.approx(21.) + a, b, c, d, e, f, g, h, j, k = s._get_base_coeffs() + assert (a, b, c) == pytest.approx((1., 1., 0.)) + assert (d, e, f) == pytest.approx((0., 0., 0.)) + assert (g, h, j) == pytest.approx((-6., -8., 0.)) + assert k == pytest.approx(21.) # (y + 7)^2 + (z - 1)^2 = 3^2 # y^2 + z^2 + 14y - 2z + 41 = 0 s = openmc.model.cylinder_from_points((0., -7, 1.), (1., -7., 1.), 3.) - assert (s.a, s.b, s.c) == pytest.approx((0., 1., 1.)) - assert (s.d, s.e, s.f) == pytest.approx((0., 0., 0.)) - assert (s.g, s.h, s.j) == pytest.approx((0., 14., -2.)) - assert s.k == 41. + a, b, c, d, e, f, g, h, j, k = s._get_base_coeffs() + assert (a, b, c) == pytest.approx((0., 1., 1.)) + assert (d, e, f) == pytest.approx((0., 0., 0.)) + assert (g, h, j) == pytest.approx((0., 14., -2.)) + assert k == 41. # (x - 2)^2 + (z - 5)^2 = 4^2 # x^2 + z^2 - 4x - 10z + 13 = 0 s = openmc.model.cylinder_from_points((2., 0., 5.), (2., 1., 5.), 4.) - assert (s.a, s.b, s.c) == pytest.approx((1., 0., 1.)) - assert (s.d, s.e, s.f) == pytest.approx((0., 0., 0.)) - assert (s.g, s.h, s.j) == pytest.approx((-4., 0., -10.)) - assert s.k == pytest.approx(13.) + a, b, c, d, e, f, g, h, j, k = s._get_base_coeffs() + assert (a, b, c) == pytest.approx((1., 0., 1.)) + assert (d, e, f) == pytest.approx((0., 0., 0.)) + assert (g, h, j) == pytest.approx((-4., 0., -10.)) + assert k == pytest.approx(13.)