From ad3a14fa3d4ae8f250913a5221ad808d92c33b62 Mon Sep 17 00:00:00 2001 From: agnelson Date: Fri, 30 Jul 2021 11:42:27 -0500 Subject: [PATCH] Resolving comments from @paulromano. Namely, set the openmc.lib.Cell rotation and translations to be properties that can take iterables vice ndarrays. Also removed const declaration when not needed. Finally, revised code and tests so that we do not return a None for non-initialized cases. This would yield different behavior between translation and rotation and instead of leading to confusion I now simply always return a sensical ndarray of float values.That is no rotation is no longer None, but is now array([0., 0., 0.]). --- include/openmc/capi.h | 2 +- openmc/lib/cell.py | 68 +++++++++++++++--------------------- src/cell.cpp | 6 ++-- tests/unit_tests/test_lib.py | 20 +++++------ 4 files changed, 42 insertions(+), 54 deletions(-) diff --git a/include/openmc/capi.h b/include/openmc/capi.h index 262bae6d79..012c7c18a0 100644 --- a/include/openmc/capi.h +++ b/include/openmc/capi.h @@ -23,7 +23,7 @@ extern "C" { int openmc_cell_set_id(int32_t index, int32_t id); int openmc_cell_set_temperature(int32_t index, double T, const int32_t* instance, bool set_contained = false); int openmc_cell_set_translation(int32_t index, const double xyz[]); - int openmc_cell_set_rotation(int32_t index, const double rot[], const size_t rot_len); + int openmc_cell_set_rotation(int32_t index, const double rot[], size_t rot_len); int openmc_energy_filter_get_bins(int32_t index, const double** energies, size_t* n); int openmc_energy_filter_set_bins(int32_t index, size_t n, const double* energies); int openmc_energyfunc_filter_get_energy(int32_t index, size_t* n, const double** energy); diff --git a/openmc/lib/cell.py b/openmc/lib/cell.py index dd522d614e..b53a6be7a7 100644 --- a/openmc/lib/cell.py +++ b/openmc/lib/cell.py @@ -103,6 +103,13 @@ class Cell(_FortranObjectWithID): Number of unique cell instances bounding_box : 2-tuple of numpy.ndarray Lower-left and upper-right coordinates of bounding box + translation : Iterable of float + 3-D coordinates of the translation vector + rotation : Iterable of float + The rotation matrix or angles of the universe filling the cell. This + can either be a fully specified 3 x 3 rotation matrix or an Iterable + of length 3 with the angles in degrees about the x, y, and z axes, + respectively. """ __instances = WeakValueDictionary() @@ -227,62 +234,43 @@ class Cell(_FortranObjectWithID): _dll.openmc_cell_set_temperature(self._index, T, instance, set_contained) - def get_translation(self): - """Get the translation vector of a cell - - """ - + @property + def translation(self): translation = np.zeros(3) _dll.openmc_cell_get_translation( self._index, translation.ctypes.data_as(POINTER(c_double))) return translation - def set_translation(self, translation): - """Set the translation vector of a cell - - Parameters - ---------- - translation : numpy.ndarray - 3-D translation vector - - """ - + @translation.setter + def translation(self, translation_vec): + vector = np.asarray(translation_vec, dtype=float) _dll.openmc_cell_set_translation( - self._index, translation.ctypes.data_as(POINTER(c_double))) + self._index, vector.ctypes.data_as(POINTER(c_double))) - def get_rotation(self): - """Get the rotation matrix of a cell - - """ - - rotation = np.zeros(12) + @property + def rotation(self): + rotation_data = np.zeros(12) rot_size = c_size_t() _dll.openmc_cell_get_rotation( - self._index, rotation.ctypes.data_as(POINTER(c_double)), rot_size) + self._index, rotation_data.ctypes.data_as(POINTER(c_double)), + rot_size) rot_size = rot_size.value - if rot_size == 0: - return None - elif rot_size == 9: - return rotation[:n] - elif rot_size == 12: - return rotation[9:] + if rot_size == 9: + return rotation_data[:rot_size].shape(3, 3) + elif rot_size in (0, 12): + # If size is 0, rotation_data[9:] will be zeros. This indicates no + # rotation and is the most straightforward way to always return + # an iterable of floats + return rotation_data[9:] else: raise ValueError( 'Invalid size of rotation matrix: {}'.format(rot_size)) - def set_rotation(self, rotation): - """Set the rotation matrix of a cell - - Parameters - ---------- - rotation : numpy.ndarray - The rotation angles (if length is 3) or the 3x3 rotation matrix - - """ - - flat_rotation = rotation.flatten() + @rotation.setter + def rotation(self, rotation_data): + flat_rotation = np.asarray(rotation_data, dtype=float).flatten() _dll.openmc_cell_set_rotation( self._index, flat_rotation.ctypes.data_as(POINTER(c_double)), diff --git a/src/cell.cpp b/src/cell.cpp index dccae9bea7..97f64e3a1d 100644 --- a/src/cell.cpp +++ b/src/cell.cpp @@ -1309,7 +1309,7 @@ openmc_cell_set_id(int32_t index, int32_t id) extern "C" int openmc_cell_get_translation(int32_t index, double xyz[]) { if (index >= 0 && index < model::cells.size()) { - auto& cell = openmc::model::cells[index]; + auto& cell = model::cells[index]; xyz[0] = cell->translation_.x; xyz[1] = cell->translation_.y; xyz[2] = cell->translation_.z; @@ -1342,7 +1342,7 @@ extern "C" int openmc_cell_set_translation(int32_t index, const double xyz[]) extern "C" int openmc_cell_get_rotation(int32_t index, double rot[], size_t* n) { if (index >= 0 && index < model::cells.size()) { - auto& cell = openmc::model::cells[index]; + auto& cell = model::cells[index]; *n = cell->rotation_.size(); std::memcpy(rot, cell->rotation_.data(), *n * sizeof(cell->rotation_[0])); return 0; @@ -1354,7 +1354,7 @@ extern "C" int openmc_cell_get_rotation(int32_t index, double rot[], size_t* n) //! Set the flattened rotation matrix of a cell extern "C" int openmc_cell_set_rotation(int32_t index, const double rot[], - const size_t rot_len) + size_t rot_len) { if (index >= 0 && index < model::cells.size()) { if (model::cells[index]->fill_ == C_NONE) { diff --git a/tests/unit_tests/test_lib.py b/tests/unit_tests/test_lib.py index 5566874b02..41613832f3 100644 --- a/tests/unit_tests/test_lib.py +++ b/tests/unit_tests/test_lib.py @@ -717,28 +717,28 @@ def test_cell_translation(pincell_model_w_univ, mpi_intracomm): # Cell 1 is filled with a material so it has a translation, but we can't # set it. cell = openmc.lib.cells[1] - assert cell.get_translation() == pytest.approx([0., 0., 0.]) + assert cell.translation == pytest.approx([0., 0., 0.]) with pytest.raises(exc.GeometryError, match='not filled with'): - cell.set_translation(np.array([1., 0., -1.])) + cell.translation = (1., 0., -1.) # Cell 2 was given a universe, so we can assign it a translation vector cell = openmc.lib.cells[2] - assert cell.get_translation() == pytest.approx([0., 0., 0.]) + assert cell.translation == pytest.approx([0., 0., 0.]) # This time we *can* set it - cell.set_translation(np.array([1., 0., -1.])) - assert cell.get_translation() == pytest.approx([1., 0., -1.]) + cell.translation = (1., 0., -1.) + assert cell.translation == pytest.approx([1., 0., -1.]) def test_cell_rotation(pincell_model_w_univ): # Cell 1 is filled with a material so we cannot rotate it, but we can get # its rotation matrix (which will be the identity matrix) cell = openmc.lib.cells[1] - assert cell.get_rotation() is None + assert cell.rotation == pytest.approx([0., 0., 0.]) with pytest.raises(exc.GeometryError, match='not filled with'): - cell.set_rotation(np.array([180., 0., 0.])) + cell.rotation = (180., 0., 0.) # Now repeat with Cell 2 and we will be allowed to do it cell = openmc.lib.cells[2] - assert cell.get_rotation() is None - cell.set_rotation(np.array([180., 0., 0.])) - assert cell.get_rotation() == pytest.approx([180., 0., 0.]) + assert cell.rotation == pytest.approx([0., 0., 0.]) + cell.rotation = (180., 0., 0.) + assert cell.rotation == pytest.approx([180., 0., 0.])