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

[wip] add MeasurementND classes #170

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

tmadlener
Copy link
Contributor

@tmadlener tmadlener commented Aug 8, 2022

BEGINRELEASENOTES

  • TODO

ENDRELEASENOTES

This is a draft implementation for the MeasurementND classes mentioned in #161. In its current form it only implements a two dimensional example, but it should easily scale to other numbers of dimensions as well. It probably has some potential for generalizing the ExtraCode bits slightly for that.

In the current form it introduces a Measurement2D class that has three std::arrays internally. One for the central values, one for the covariance matrix and one that stores "meta" information on the dimensions. The dimensions in this case are enums (enum class also works). The meta information stores which dimension corresponds to which stored value and there is functionality to set/get the order of the dimensions. Using ExtraCode we essentially simply layer a second interface that grants access to the stored values via the dimensions directly. Potentially simpler shown than told:

enum Cartesian : edm4hep::DimType { X = 0, Y, Z}; // internally we use uint16_t, DimType is just a typedef for that

auto m = MutableMeasurement2D();
// This measurement should describe one in a (Z, X) space
m.setDimensions(Cartesian::Z, Cartesian::X);
// Setting the value for Z
m.setValue(3.14f, Cartesian::Z);
// Setting a value in the covariance matrix (order of dimensions doesn't matter)
m.setCov(1.23f, Cartesian::X, Cartesian::Z);

Retrieving the values can be done similarly.

One thing that is not even attempted in the current version is to guard against access with a different dimension enum, e.g. using the above m the following works without problems

enum class Polar { r, theta, phi };
// this DOES NOT have to be the same dimensions as above
const auto [dim1, dim2] = m.getDimensions<Polar>();
// dim1 == Polar::phi
// dim2 == Polar::r

auto val = m.getValue(Polar::phi);
// val == 3.14f

Since this interface is simply added to the one that is generated, the whole class is also usable without any specific enums.

@paulgessinger does this go into a direction that is useful for ACTS?

TODO:

  • Geometry info / CellID field
  • Error handling. What should happen if access with a dimension that is not stored is attempted?
  • Other Ns
  • Better naming of things?
  • Release notes

should exhibit all the complexities for ND
@tmadlener tmadlener marked this pull request as draft August 10, 2022 15:11
@paulgessinger
Copy link
Contributor

I realize I never commented on this, sorry.
Thanks @tmadlener, at first glance, this looks useful for us. I haven't had time to look at it in more detail and test it out, but I plan to get back to this soon.

@wdconinc
Copy link
Contributor

I'm curious about this PR. Is this still of interest or is it being pursued elsewhere?

@tmadlener
Copy link
Contributor Author

As far as I am aware, there is still interest in this, but there hasn't been any work recently. From the podio side there might be a few technical issues to solve, e.g. how to store measurements with different dimensionality (e.g. pixel and strip hits) in one collection (at least in memory).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants