Skip to content
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

Merged
merged 9 commits into from Jan 30, 2020

Conversation

randlet
Copy link
Contributor

@randlet randlet commented Jun 9, 2016

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).

@ftessier
Copy link
Member

ftessier commented Jun 28, 2016

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.

@ftessier ftessier self-assigned this Jun 28, 2016
@ftessier ftessier modified the milestone: Release 2017 Jun 28, 2016
@randlet
Copy link
Contributor Author

randlet commented Jun 29, 2016

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?

@ftessier
Copy link
Member

ftessier commented Jun 29, 2016

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.

@randlet
Copy link
Contributor Author

randlet commented Jun 30, 2016

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.

@mchamberland
Copy link
Contributor

@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:
Line 35 should be:
link2_libs = egs_gtransformed egspp z

egs_glib:
Line 25 should be:
link2_libs = egspp egs_ndgeometry egs_planes z

@mchamberland
Copy link
Contributor

Or maybe a better way is having the following in the original Makefiles (using the egs_autoenvelope Makefile as an example):

GZSTREAM =
GZSTREAM_DEF =
GZSTREAM_H =
GZSTREAM_LIB =
ifneq ("$(wildcard gzstream.cpp)","")
GZSTREAM = gzstream
GZSTREAM_DEF = -DBUILD_GZSTREAM -DHAS_GZSTREAM
GZSTREAM_H = gzstream.h
GZSTREAM_LIB = z
endif

DEFS = $(DEF1) -DBUILD_AENVELOPE_DLL $(SOBOL_DEF) $(GZSTREAM_DEF)


library = egs_autoenvelope
lib_files = egs_autoenvelope $(SOBOL) $(GZSTREAM)

link2_libs = egs_gtransformed egspp $(GZSTREAM_LIB)

@mchamberland
Copy link
Contributor

mchamberland commented Jul 13, 2016

@randlet Also, I think line 43 in egs_autoenvelope/Makefile should be:

$(DSO2)autoenvelope.$(obje): volcor.h $(GZSTREAM_H) \

so, remove the $(SOBOL_H) part. With the $(SOBOL_H) part, the sobol.o file does not get created under the egs++/dso folder, which leads to compilation errors elsewhere. Removing the $(SOBOL_H) seems to resolve this.

@randlet
Copy link
Contributor Author

randlet commented Jul 15, 2016

Thanks @mchamberland :)

@randlet
Copy link
Contributor Author

randlet commented Nov 7, 2016

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);
Copy link
Contributor

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) {
Copy link
Contributor

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.

@ftessier
Copy link
Member

Reorganized and squashed a little further. Still no diff with the original randlet/feature-geometries.

@ftessier ftessier force-pushed the feature-geometries branch 2 times, most recently from e27e5e9 to 6c2087f Compare January 23, 2020 17:33
@ftessier
Copy link
Member

No changes still. I had just messed up authorship while fixing merge conflicts with gitkraken.

@ftessier
Copy link
Member

This does not compile since it was rebased on develop, because of pull request #364 after this pull request was created, and changed the way the projectors are called.

@ftessier
Copy link
Member

Fixed the ZProjector compilation error, and tweaked typos and format slightly.

@ftessier
Copy link
Member

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 egs_cylinders.cpp to the small-stuff branch. The diff with the original branch does not otherwise show any functional code changes. Commit messages are all messed up from all the squashing and reordering; will update them now.

@ftessier
Copy link
Member

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.

@ftessier
Copy link
Member

ftessier commented Jan 30, 2020

This branch compiles without errors, and I checked that I can run an autoenvelope simulation, and load in egs_view the example HEN_HOUSE/egs++/geometry/examples/seeds_in_xyz_aenv.geom. Hence I am ready to merge this into develop, pending last review.

ftessier and others added 9 commits January 30, 2020 00:17
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.
@ftessier
Copy link
Member

Rebased on develop.

@ftessier
Copy link
Member

@rtownson is the segmentation fault resolved at your end?

@rtownson
Copy link
Contributor

@ftessier Yes, it's possible that the rebase fixed the segmentation fault issue. I'm ok with merging this in.

@ftessier ftessier merged commit f3e94ec into nrc-cnrc:develop Jan 30, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants