Refactor CSG

[lmh91]

I started refactoring our CSG. The core is finished. See branch refactorCSG.

This also concerns #62, #63, #84

The CSG is separated in a new submodule ConstructiveSolidGeometry inside SSD.

Rotation, translation and stretching/scaling are now implemented.

For now, I only added one primitive, the Tube.

I added a test script under /test/ConstructiveSolidGeometry/CSG_test.jl.
At the moment is more an example script.

We can start implementing our other primitives and functionality, like read-in, to it or adept to it.
But when adding something, please test for performance (type stability) like it is done in the test script.
A PR to this branch must have tests.

Guideline for Primitives

• Our primitives will be parametric types so that we can save computation time for special version of the primitives.
  E.g., in case of a full tube (cylinder) we don't have to check for rMin and the polar coordinate.

• Default constructors: All primitives should be constructed around (0,0,0). This makes rotations easier for the user. This is also the way how it is done in GEANT-4. E.g., Tube(radius = 1.0, height = 1.0) will create a Tube with an radius of 1 and going from -0.5 to +0.5 in z. Here, we will have to change (and break) our read-in our config files.

General remarks

• Branch refactorCSG: The old CSG is still inside the package. The new one is only included but not used yet.
  CartesianPoint and CylindricalPoint are not exported anymore to avoid conflicts as there exists now two version (for now). E.g.: SolidStateDetectors.CartesianPoint and SolidStateDetectors.ConstructiveSolidGeometry.CartesianPoint.

• The conversion of a CartesianPoint into a CylindricalPoint got changed.
  Previously: r = sqrt(pt.x * pt.x + pt.y * pt.y
  New: r = hypot(pt.x, pt.y) (special algorithm, see Julia documentation of hypot)

ToDo:

EDIT May 15th, 2021
The primitives have all been updated. Find a list of what still needs to be done at the end of this issue.

[fhagemann]

Repost from #141:

What still needs to be done:

• SurfacePrimitives and respective functions for ALL VolumePrimitives (#147, #150, #153, #156)
• Proper handling of CSGUnion, CSGDifference and CSGIntersection, as well as their decomposition into surfaces.
• Proper handling of transformations other than translations in z
• Config files and read in of StretchedGeometry and RotatedGeometry (#155)
• Adding distance_to_surface methods (#154, #159)
• Plotting electric field lines and detectors with :samplesurface (#170)

[lmh91]

Okay, I finally had some time looking into the code and have so the next days.

So far I have one remark:

Constructive Solid Geometry and volume primitives already implies that those primitives are not surfaces.
So, a Box will never have no expansion in one dimension. If so, it should be turned into an surface primitive.
This should happen in the construction and not anymore in functions handling those primitives, e.g. in get_decomposed_surfaces(b::Box). Also, functions like _is_torus_collapsed are also obsolete,
because this "volume primitive" is actually than a "line primitive".

This also solves the problem with the type instability for the function get_decomposed_surfaces.
This function should return a tuple of a fixed number (depending on the volume primitive)
of surfaces primitives, which makes it type stable. E.g. get_decomposed_surfaces(b::Box) will always return 6 rectangles.

I will continue to look through everything.

[lmh91]

Another point: :closed vs :open primitives.
I think @fhagemann and I talked about this some time ago.

Its about whether the surface of a primitives belongs to the primitive or not.
This is needed for differences in CSGs.
It's a definition but we should agree on how to handle it.
And in my opinion I would like to include the intersection surface of the two primitives in their difference:
A - B -> A::Closed - Open(B) (I hope that syntax is clear here)

So all primitives would need an additional parametric type field. I would suggest to always use it as the second one as all primitives will have it.
E.g. Sphere{T,TR} will change to Sphere{T,CO,TR} (CO for ClosedOpen but it does not matter).
We can either use :closed or :open or add introduce types for it.

[lmh91]

Now that all primitives have a origin and a rotation I would like to remove the from-to syntax for certain case from the configuration file format.

E.g. in case of the box primitives:

box:
  x:
    from: -1.0
    to: 1.0
  y:
    from: -2.0
    to: 2.0
  z:
    from: -3.0
    to: 3.0

I would suggest to not allow that syntax anymore because it influences the origin
and instead go for something like:

box:
  widths: [2, 4, 6] # hX -> 2/2 = 1;  hY -> 4/2 = 2; hZ -> 6/2 = 3;
  origin: [0, 0, 0] # -> CartesianPoint{T}(0, 0, 0)

And also allow a possible rotation with the remaining rule that the rotation is performed before the translation.

With this I would also remove the z field which was a translation in z before.
E.g. in the Torus example file:

torus:
  r_torus: 10.0
  r_tube:
      from: 2.0
      to: 2.0
  phi: 
      from: 0.0°
      to: 360.0°
  theta: 
      from: 0.0°
      to: 360.0°
  z: 0.0

For the angles or intervals like for r_in & r_out I would still keep the from-to syntax as, here, it does not influence the origin.

So in general I would go for:

primitive:
   <specific fields of the primitive>: ...
   origin: [x, y, z]
   rotation: <current rotation notation>

What do you think?

[fhagemann]

Why not allow for both?
The from-to syntax can be easily "translated" to the current implementation of primitives?

[lmh91]

Yes, the from-to syntax works right now also with the new version of the primitives.

I just find the definition with specific origin and rotation fields clearer
as both might lead to confusions:

box:
  x:
    from: -1.0
    to: 1.0
  y:
    from: -2.0
    to: 2.0
  z:
    from: -3.0
    to: 3.0
  origin: [3, 3, 3] 

Where should the origin now be? Sure, one could specify some rules how they are "ordered" but this might cause some confusion.

Prior to now we had just a z field for a translation in z. But now we have arbitrary translations so I would like to introduce the origin-field. Just adding x and y would also do but it might be unclear what it means and might conflict with other fields of
certain primitives.

[lmh91]

The cone can also be defined via z:

cone:
  r:
    bottom:
      from: 1.0
      to: 2.0
    top:
      from: 1.0
      to: 4.0
  phi:
    from: 0.0
    to: 6.283185307179586
  h: 2.0
  z: 
    from: 0
    to: 3

Here, z is ignored and h is used to define the cone from -1 to +1 in z.

If h does not exists z is used to define it from 0 to 3 in z.

[lmh91]

I agree with your first point. I just named your translate inside the primitive definition origin.

But I think x, y and z should never represent any primitives.
E.g., in case for the cone, z is bad description for the height (-> h is far better/clearer) of the primitive.
And for the box just x, y and z are also a bit unclear as we also allow the syntax

box:
  x: 1 # width in x: from -0.5 to +0.5 in x but the user might think it is -1 to 1
  y: 2
  z: 3

widhts or halfwidhts or hX, hY, hZ would be much clearer together in combination with origin.
We can also add the syntax

origin:
  z: 2

which would be identical to origin: [0,0,2].

Outside of the core primitive definition we still can wrap everything in translations and rotations
via translate and rotate like we currently do. There I don't want to change anything.

[fhagemann]

Please feel free to edit this comment:

For merging v0.6 onto master

• Decide for a final config file format
• Documentation

What still needs to be done for the v0.6 release:

• Enhance refinement (will change syntax of some functions like simulate! and calculate_electric_potential!)
• Adjust Simulation struct to match the config file format
• Make sure all functionalities of the old v0.5 SSD work again
• Refactor tests (Coax should not be simulated for each test run)
• Write precompilation statements to avoid long compilation times as in Plots.jl
• Clean-up

Optional, e.g. v0.6.1

• Make apply_initial_state!() allocation free and performant
• Reduce allocations / increase performance wherever possible
• Restrict the parametric types to specific types

Optional (would go into v0.7)

• distance_to_surface
• plot(:sample_surface)
• Refactor drift code and get this allocation-free