Cell Rotations and Translations #225

wbinventor · 2015-11-23T03:45:35Z

This PR introduces cell rotations and translations. These features enable us to model the fully-detailed (3D) BEAVRS benchmark generated by the OpenCG BEAVRS builder for the first time!

Unfortunately, adding in rotations/translations was a little more difficult than I had expected (but not overly cumbersome). Fortunately, the process of putting in these features required some refactoring of the ray tracing code - but it keeps getting better and better and is far more methodical than a few years ago.

In particular, this PR introduces length 3 _rotation and _translation arrays for the Cell class which may be optionally set by the user. The ray tracing code in the Geometry and Universe classes has been refactored such that all updates to LocalCoords positions take place directly within the LocalCoords object (which is a cleaner abstraction than before). In addition, the 2D angle phi wrt the x-axis is now an attribute of LocalCoords, where it is defined based on a track's starting angle and the rotations applied to any Cells found during CSG tree traversal.

With the changes made here, it is quite simple to build a full-core OpenMOC model of the 3D BEAVRS core. In particular, with an installation of the BEAVRS model builder from the mit-crpg/PWR_benchmarks repo, along with OpenCG (a pre-requisite for the BEAVRS builder), one can instantiate an OpenMOC model of BEAVRS as follows:

import opencg
from beavrs.builder import BEAVRS
from openmoc.compatible import get_openmoc_geometry

# Instantiate a BEAVRS OpencG model from mit-crpg/PWR_benchmarks
beavrs = BEAVRS(nndc_xs=True)

# Construct an OpenCG Geometry for the BEAVRS reactor model
opencg_geometry = opencg.Geometry()
opencg_geometry.root_universe = beavrs.main_universe

# Export the 3D OpenCG Geometry to OpenMOC
openmoc_geometry = get_openmoc_geometry(opencg_geometry)

# Voila!

Previously, if one attempted to export the model as above, the baffles were not placed in the correct positions. This is because the baffles have rotations applied. Now that the baffles can be properly treated, the model above can be plotted as shown below and the baffles look as they should.

Full core

Upper right core

Note that the OpenMOC Geometry created above will not have a bounding box applied, and hence is not yet ready for ray tracing. In order to make the Geometry "ray tracing-ready", one would must create a Cell outside the core vessel filled by a dummy Material and bounded by the core vessel's ZCylinder and some bounding XPlanes and YPlanes with VACUUM boundary conditions.

…rial / cell ID

…/ rotations can now be converted to OpenCG

…y tracing

…ces. Rotations need to be updated.

…lattices

…refactor

…ines

samuelshaner · 2015-11-23T13:59:51Z

@wbinventor - this is great! I'll give this a full review later this week.

samuelshaner · 2015-11-28T17:42:52Z

openmoc/compatible/opencg_compatible.py

    if isinstance(fill, openmoc.Lattice):
      opencg_cell.fill = get_opencg_lattice(fill)
    else:
      opencg_cell.fill = get_opencg_universe(fill)

+  if openmoc_cell.isRotated():
+    rotation = openmoc_cell.getRotation(3)
+    opencg_cell.rotation = rotation * -1


What's the reason for the -1 here and on line 599?

Haha, it was a little hack while I was trying to figure things out. I've removed it now as I've straightened out things in the roll-pitch-yaw convention of the rotation matrix below.

samuelshaner · 2015-11-28T19:09:48Z

The rotations are input and stored in degrees, but angles are for the most part handled internally in radians. The Cell and LocalCoords objects both have getPhi() methods, but Cell returns phi in degrees while LocalCoords returns phi in radians. At the surface, this can be a bit confusing. I think handling angles internally in one common set of units (I think radians would be most appropriate) would make the code easier to understand and avoid any conversion errors down the road, but allowing the user to input angles for rotations in degrees or radians (which a function parameter indicating what units are being input) would be a reasonable compromise from a user-input standpoint. What are your thoughts on the radians/degrees mixing in the code?

samuelshaner · 2015-11-28T19:13:05Z

src/Universe.cpp

+	  next_coords->setX(new_x);
+	  next_coords->setY(new_y);
+	  next_coords->setZ(new_z);
+          next_coords->incrementPhi(cell->getPsi() * M_PI / 180.);


indented one too many spaces

Weird, it doesn't show up that way in emacs. I removed all whitespace and added it back in so perhaps it will show up properly now.

…schemes in OpenMOC/CG

wbinventor · 2015-12-01T03:51:24Z

I've addressed all of the comments made thus far and think this is ready for a final review.

samuelshaner · 2015-12-01T19:30:07Z

Thanks for updating the PR @wbinventor. I had a comment on mixing of angle units (radians vs degrees) in the code that it looks like you might have missed.

…hods

wbinventor · 2015-12-02T00:15:16Z

@samuelshaner - the rotation angles are now stored internally in radians. The setter and getter routines for the angles now accept a std::string units parameter which defaults to "degrees" but which may alternatively accept "radians".

samuelshaner · 2015-12-06T18:56:42Z

openmoc/numpy_typemaps.i

+
+/* The typemap used to match the method signature for the Cell's
+ * getter method for rotations used by the OpenCG compatibility module. */
+%apply (double* ARGOUT_ARRAY1, int DIM1) {(double* rotations, int num_axes)}


does this signature need another attribute for the units?

I wondered that too, but it doesn't seem too since it works with my test case.

Hmmm...maybe swig just ignores all arguments after those included in the signature.

That's what I think. But this is related to the way NumPy typemaps work with SWIG - and that isn't particularly well documented IMHO.

samuelshaner · 2015-12-06T21:40:43Z

Just tested this out and everything looks good to merge!

Cell Rotations and Translations

Will Boyd added 18 commits November 21, 2015 17:10

Fixed bug in plotter module when plotting void regions without a mate…

3bca812

…rial / cell ID

Initial implementation of rotation attribute and setter in Cell class

bcc800c

Cell now computes 3x3 rotation matrix when rotation angles are specified

2544255

Cell clone now copies rotation angles if any

b4db6b6

Rotations now working for plotting but not yet track generation

0d834c8

Fixed rotations - now working for full core BEAVRS!!!

937bb55

Fixed a few bugs in OpenCG compatibility module: OpenMOC geometries w…

c74105e

…/ rotations can now be converted to OpenCG

Began refactoring code to apply rotations to angular directions in ra…

897069f

…y tracing

Ray tracing code now semi-intact - working for pin cell but not latti…

3095060

…ces. Rotations need to be updated.

Cleanup of refactored ray tracing code, now working for pin cell and …

7487920

…lattices

Rotations now appear to be working during ray tracing!!!

fe0b8de

Restored pin cell sample input file

7359765

Cleaned up application of rotations in Universe::findCell()

477c58f

Restored simple-lattice.py input file

4018686

Cleaned up doxygen comments for class methods touched by ray tracing …

05d693e

…refactor

Initial implementation of cell translations with getter / setter rout…

a233bb0

…ines

Added translations to ray tracing

15a31d9

Added translations to OpenCG compatibility module

8a000a3

wbinventor added Geometry Ray Tracing labels Nov 23, 2015

wbinventor assigned samuelshaner Nov 23, 2015

wbinventor mentioned this pull request Nov 25, 2015

Redundant surfaces #226

Open

samuelshaner reviewed Nov 28, 2015
View reviewed changes

wbinventor changed the title ~~Cells Rotations and Translations~~ Cell Rotations and Translations Nov 29, 2015

wbinventor@gmail.com added 4 commits November 30, 2015 22:18

Fixed doxygen comment returns -> return

8218237

Clarified comments and code for roll-pitch-yaw rotation convention

5b06cb3

Addressed comments by @samuelshaner and @geogunow

5c93374

Reverted comment in OpenCG compatibility module for lattice indexing …

2b31945

…schemes in OpenMOC/CG

wbinventor@gmail.com added 2 commits December 1, 2015 18:57

Now store rotation angles in radians per comments by @samuelshaner

64f4dc5

Added string units parameter to Cell rotation getters and setters met…

0f34157

…hods

samuelshaner reviewed Dec 6, 2015
View reviewed changes

samuelshaner added a commit that referenced this pull request Dec 6, 2015

Merge pull request #225 from wbinventor/rotation-translation

479cd68

Cell Rotations and Translations

samuelshaner merged commit 479cd68 into mit-crpg:develop Dec 6, 2015

wbinventor deleted the rotation-translation branch December 6, 2015 22:04

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Cell Rotations and Translations #225

Cell Rotations and Translations #225

wbinventor commented Nov 23, 2015

samuelshaner commented Nov 23, 2015

samuelshaner Nov 28, 2015

wbinventor Dec 1, 2015

samuelshaner commented Nov 28, 2015

samuelshaner Nov 28, 2015

wbinventor Dec 1, 2015

wbinventor commented Dec 1, 2015

samuelshaner commented Dec 1, 2015

wbinventor commented Dec 2, 2015

samuelshaner Dec 6, 2015

wbinventor Dec 6, 2015

samuelshaner Dec 6, 2015

wbinventor Dec 6, 2015

samuelshaner commented Dec 6, 2015

Cell Rotations and Translations #225

Cell Rotations and Translations #225

Conversation

wbinventor commented Nov 23, 2015

samuelshaner commented Nov 23, 2015

Choose a reason for hiding this comment

Choose a reason for hiding this comment

samuelshaner commented Nov 28, 2015

Choose a reason for hiding this comment

Choose a reason for hiding this comment

wbinventor commented Dec 1, 2015

samuelshaner commented Dec 1, 2015

wbinventor commented Dec 2, 2015

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

samuelshaner commented Dec 6, 2015