-
Notifications
You must be signed in to change notification settings - Fork 146
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Additional egs++ geometries and shapes #103
Conversation
Great work @randlet, these are all extremely useful addition to the egs++ library! We cannot include in the project anything over which NRC does not own copyright. How critical are gzstream and sobol to this PR? We don't want to squash larger PR such as this one! We want to preserve your commit history and add a proper merge commit to develop. Typically we only squash smaller changes, so as to limit the number of tiny merge bubbles. We will probably take some time to review, but I am tagging it for the next release. |
The gzstream and Sobol RNG code is for efficiency (disk space in the case of gzstream and cpu time in the case of Sobol) and not crucial to any of the libraries. Perhaps I could make use of preprocessor directives and allow people to conditionally compile those features after they download the gzstream & Sobol RNG themselves? |
Sounds like a good idea: leave it as an option and provide instructions to load the third party libraries. We have some sobol code, so perhaps we can use that. |
2be07c1
to
4900ebc
Compare
I've updated this so that everything will now compile and run without the gzstream & Sobol rng dependencies. Users who want the extra functionality can obtain the code required from here. |
@randlet if a user wants to install the gzip functionality on Mac, the egs_autoenvelope and egs_glib Makefiles need to explicitly link against the 'z' library to compile. So in your repository for extra functionality, you should include modified Makefiles for auto envelope and glib. egs_autoenvelope: egs_glib: |
Or maybe a better way is having the following in the original Makefiles (using the egs_autoenvelope Makefile as an example):
|
@randlet Also, I think line 43 in egs_autoenvelope/Makefile should be:
so, remove the |
Thanks @mchamberland :) |
Thanks for all the review @rtownson! Can you ping me when you've finished your initial review and I'll get everything updated. |
vector<EGS_Float> getUncorrectedVolumes(EGS_BaseGeometry *base) { | ||
vector<EGS_Float> uncorrected; | ||
for (int ir=0; ir < base->regions(); ir++) { | ||
EGS_Float vol = base->getMass(ir)/base->getRelativeRho(ir); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not all geometries have getMass
! For example, egs_box
.
What happens if we can't calculate the mass for the base geometry, because it is complex? Maybe there should be a check and return an error for geometries that are not supported.
* the volume of those regions. The algorithm is described | ||
* on main page of the Auto Envelope documentation. */ | ||
VCResults findRegionsWithInscribed(VCOptions *opts, EGS_BaseGeometry *base, | ||
vector<EGS_BaseGeometry *> inscribed, vector<EGS_AffineTransform *> transforms) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should look into making use of the more efficient volume calculations in pull request #184.
Reorganized and squashed a little further. Still no diff with the original |
e27e5e9
to
6c2087f
Compare
No changes still. I had just messed up authorship while fixing merge conflicts with gitkraken. |
This does not compile since it was rebased on |
6c2087f
to
430205c
Compare
Fixed the ZProjector compilation error, and tweaked typos and format slightly. |
430205c
to
d20bc7a
Compare
Condensed into 9 "orthogonal" commits: they now all pertain to a single directory. Squashed style and comments commits. Separated out the glib from the auto envelope commits (which made me the commit author unfortunately). Ported the small commit to curb the compiler warning for |
d20bc7a
to
22df066
Compare
Updated the commit messages according to the new commit line. Also updated the copyright and contributors for the new sample input files to reflect the ones found in the corresponding source code additions. Verified the diff with the original branch. |
This branch compiles without errors, and I checked that I can run an autoenvelope simulation, and load in egs_view the example |
The egs_glib geometry can load a geometry which is defined in an external file, allowing geometry definitions to be modularized across multiple files, reused, shared, etc. There are options to compile with gzip capability (for egs_glib and egs_autoenvelope), and to compile with a Sobol random number generator (for the egs_autoenvelope region discovery sampling). For either of these features, install the egspp-geometry-lib-extras from: https://github.com/clrp-code/egspp-geometry-lib-extras/
Implement a new spherical shell geometry which has a hollow central region, considered to be outside the shell geometry. Also implement getBound, getNRegDir, and getMass methods for spheres.
Add a new geometry to create a cylindrical (rz) geometry, for example, a cylindrical phantom. Internally it relies on the nd geometry, but it is more convenient than defining it from scratch with cylinders and planes.
Add a shape which is analogous to a conestack geometry, with the exception that the top and bottom radii accept only 1 or 2 values. A single input is taken as the outer radius (the inner radius is then 0), whereas 2 inputs define the inner and outer radii of the shell.
Add a spherical shell shape, where sampling can also be restricted to a hemisphere or, more generally, to a conical section specified by its half-angle.
Add an efficient envelope geometry, inspired by EGS_FastEnvelope. The defining feature here is that the base geometry regions containing the inscribed geometries are discovered automatically by Monte Carlo sampling. Also add a derived class EGS_ASwitchedEnvelope, which can further enable or disable inscribed geometries (useful for stepped HDR sources, for example). Note that different geometries can serve as the base geometry, however an error is returned when a full volume correction is requested and the geometry does not implement the getMass method.
Add support for egsphant files which have been gzipped. This feature becomes available at compile time if gzstream.cpp and gzstream.h are found in the egs_ndgeometry directory. The gzstream source files are available from: https://github.com/clrp-code/egspp-geometry-lib-extras/ Also document how to initialize an nd geometry from an egsphant file.
Add sample input files for glib, rz, and auto envelope geometries, as well as an nd geometry initialized with an egsphant file and ramp.
22df066
to
e6b0131
Compare
Rebased on |
@rtownson is the segmentation fault resolved at your end? |
@ftessier Yes, it's possible that the rebase fixed the segmentation fault issue. I'm ok with merging this in. |
As discussed previously this PR introduces a number of new egs++ geometries and shapes outined below (example geometry files for the new geometries are also included).
One thing of note is that there are a couple of other GPL'd files included (gzstream.cpp, gzstream.h & sobol.cpp, sobol.h) that I did not write. Hopefully that doesn't cause any issues for distribution.
Hopefully everything is in OK shape but I recognize that this is a big PR so we may have to go through a few revisions (e.g. do you want this all squashed to a single commit?).
If you need a more formal statement of transfer of copyright than those contained in commit messages then please let me know!
Geometry
EGS_AutoEnvelope
An envelope geometry that inscribes a single geometry at one or more locations within a base geometry (think brachy seeds in a phantom). The primary difference between the regular egs++ geometry envelope and the auto-envelope is that, at initialization time the auto-envelope uses a Monte-Carlo routine to discover which regions of the base geometry are occupied by the inscribed geometry. This allows us to skip bounds checking of the inscribed geometries when in regions of the base geometry that have no inscribed geometries in them. This makes the transport in the phantom (roughly) independent of the number of inscribed geometries and can give a large gain in efficiency for some simulations. This library also includes an additional EGS_ASwitchedEnvelope that allows user codes to enable / disable inscribed geometries arbitrarily (we use this for simulating inter-seed effects in TG43 calculations and stepped HDR sources).
EGS_rz
A wrapper around ND_geometry to simplify the creation of RZ geometries.
EGS_cSpheres
We've also modified the EGS_cSpheres geometry class to a) implement getBound, getNRegDir, and getMass and b) implement a spherical shell class (innermost sphere is considered to be outside the geometry).
EGS_glib
A very small shim library that allows you to load geometries from external files (similar to an "include" directive) like so:
:start geometry:
library = egs_glib
name = my_external_geom
include file = /path/to/some/external/my_geom.geom
:stop geometry:
Where the external file is a valid
geometry definition
input block. The advantages of this over a regular "include" is that you can now use egs_view to view the geometry in your external file and use "my_external_geom" in other composite geometries in your input file. This geometry can also be used to create an XYZ geometry from an egsphant file.Shapes
egs_conical_shell_stack
Allows you to define a shape in a similar way to the egs cone stack geometry.
egs_spherical_shell
Allows you to create a thin spherical shell shape. This shell can also be truncated by a conical section of a given half-angle (we use it for simulating beta-emitting eye-plaques).