Add support for new ways to configure Xregion bounds for #329

[brettviren]

• new CoordRegion and CoordBounds in util
• doctest for that new code
• integration into Drifter
• doctest for Drifter changes

[brettviren]

Hi @wenqiang-gu

I feel this is ready now. Please take a look and see if it satisfies you for #329.

[wenqiang-gu]

@brettviren I really like the new Xregion. I’ll need to test it for backward compatibility within the jsonnet configuration.

By the way, you’ve provided great examples for implementing CoordBounds. If the SBN folks have a different partitioning scheme, such as rectangular partitioning, would it make sense to declare a new class, like class CoordBoundRectPart: public CoordBounds?

[wenqiang-gu]

I conducted a quick test using pdsp/wct-sim-check.jsonnet using the following setting:

charge: -500, 
ray: params.det.bounds, 
step=0.1*wc.mm,
sn_pipes = sim.signal_pipelines, // signal only

Here is a comparison of the simulated waveforms:

Left: Waveform with the old Xregion
Right: Waveform difference between the old Xregion and the new Xregion

The difference is minimal, at most +/- 1 ADC, indicating consistent results.

[wenqiang-gu]

@brettviren I am fine with this PR, please feel free to merge.

[brettviren]

Thanks for checking! I'll merge.

By the way, you’ve provided great examples for implementing CoordBounds. If the SBN folks have a different partitioning scheme, such as rectangular partitioning, would it make sense to declare a new class, like class CoordBoundRectPart: public CoordBounds?

Yes, new types can be added.

Below I give two items of guidance that someone who is considering to add a new type should consider:

First, the CoordBoundsSampled can approximate arbitrary, single-valued surfaces. So, one should think of good motivations for making some new, more specialized type.

For example, I assume "rectangular partitioning" means that each physical cathode (for one TPC volume) has a unique offset in X from some nominal position but the cathodes are otherwise all co-parallel. This simpler geometry can be handled by the existing CoordBoundsSampled but it comes with a couple of costs that could be avoided with a new type. Namely, it would be nicer to the user to ask them to provide just the x-offsets instead of defining 4 points at the corners of each cathode. Also, the CoordBoundsRecPart could do the f(y,z)->x lookup much faster than the k-d tree (though the k-d tree is pretty fast).

If either of these two things are considered significant issues for applying the CoordBoundsSampled then there is good motivation to develop a new CoordBoundsRecPart.

Second, if a new CoordBounds type is made, the main implementation issue (besides actually defining its f(y,z)->x function) is that we must maintain a mapping from the schema of the given configuration object to the CoordBounds subclass type.

This mapping is held by make_coordbounds() and the issue here is that this mapping is somewhat implicit. That function carefully "probes" the cfg object and infers the target type based on what JSON types and attributes it finds. For each new subtype to be added we must invent a new schema for the cfg object and add some code to "probe" for it in make_coordbounds().

As long as there is no namespace collision in the JSON object attributes between the new type and all the old types there is no problem. But if the new type nominally requires the same schema as an existing type then instead the new schema must introduce some new schema element to break the ambiguity.

If we ever reach this level, I would suggest we probe for a special attribute of cfg as an object. Perhaps simply we name this attribute type which provides an explicit identifier for which CoordBounds type to target. If this attribute is omitted then it would imply CoordBoundsSampled.