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

Replacing a Handle<Segmentation> which contains a MultiSegmentation with the underlying Segmentation #1320

Open
wdconinc opened this issue Aug 26, 2024 · 2 comments
Labels
question Waiting for caller Waiting for issue opener to respond to question

Comments

@wdconinc
Copy link
Contributor

In the ePIC event reconstruction framework we store the cell dimensions as part of calorimeter hits. Depending on the type of segmentation, we store different values, so we have a selection based on auto segmentation_type = m_converter->findReadout(local).segmentation().type();. E.g.

        if (segmentation_type == "CartesianGridXY" || segmentation_type == "HexGridXY") {
            auto cell_dim = m_converter->cellDimensions(cellID);
            // etc writing X and Y only
        }

For some calorimeters we have a MultiSegmentation, and we would like to resolve the specific segmentation type of the hit before we get into our if statements.

We have implemented this with some internal implementation calls at https://github.com/eic/EICrecon/pull/1594/files. In particular, we now use the underlying detail pointers:

        const dd4hep::DDSegmentation::Segmentation* segmentation = m_converter->findReadout(local).segmentation()->segmentation;
        auto segmentation_type = segmentation->type();
        while (segmentation_type == "MultiSegmentation"){
            const auto* multi_segmentation = dynamic_cast<const dd4hep::DDSegmentation::MultiSegmentation*>(segmentation);
            // etc
            const dd4hep::DDSegmentation::Segmentation& sub_segmentation = multi_segmentation->subsegmentation(cellID);

            segmentation = &sub_segmentation;
            segmentation_type = segmentation->type();
        }

We're wondering if there's a way to handle this (no pun intended) with just Handles, without relying on internal implementation details.

@MarkusFrankATcernch
Copy link
Contributor

Hi Wouter,
not sure I correctly understand you...
In principle the segmentation handle class in DD4hep/Segmentations.h allows to access the underlying cell dimensions
in an abstract way independent from knowing the concrete implementation:

    /** \brief Returns a vector<double> of the cellDimensions of the given cell ID
     *  in natural order of dimensions, e.g., dx/dy/dz, or dr/r*dPhi
     *
     *   \param cellID cellID of the cell for which parameters are returned
     *   \return vector<double> in natural order of dimensions, e.g., dx/dy/dz, or dr/r*dPhi
     */
    std::vector<double> cellDimensions(const CellID& cellID) const;

Does this not work correctly for MultiSegmentation objects ?
If not, this is a bug, but the implementation does not suggest so:

    std::vector<double> MultiSegmentation::cellDimensions(const CellID& cID) const {
      return subsegmentation(cID).cellDimensions(cID);
    }

But clearly the meaning of the elements of the returned vector depend on the concrete sub-segmentation.

In your code link you only seem to be interested in the cellDimensions, so where is your problem?

@MarkusFrankATcernch MarkusFrankATcernch added the Waiting for caller Waiting for issue opener to respond to question label Aug 29, 2024
@MarkusFrankATcernch
Copy link
Contributor

MarkusFrankATcernch commented Nov 7, 2024

See meeting issue at https://indico.cern.ch/event/1471872

Handle<SomeSegmentation> seg = multisegmentation->subSegmentation(cellID);
or
Segmentation* seg = multisegmentation->subSegmentation(cellID);

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
question Waiting for caller Waiting for issue opener to respond to question
Projects
None yet
Development

No branches or pull requests

3 participants
@wdconinc @MarkusFrankATcernch and others