Skip to content

Add indexer method to ICalorimeterTool.#377

Open
scott-snyder wants to merge 3 commits intokey4hep:mainfrom
scott-snyder:indexer-20260222
Open

Add indexer method to ICalorimeterTool.#377
scott-snyder wants to merge 3 commits intokey4hep:mainfrom
scott-snyder:indexer-20260222

Conversation

@scott-snyder
Copy link
Contributor

Add an indexer() method to ICalorimeterTool, which returns a pointer to a new ICaloIndexer object. This provides an interface for translating cellIDs to indices.

BEGINRELEASENOTES

  • Add ICaloIndexer and ICalorimeterTool::indexer().
    ENDRELEASENOTES

Add an indexer() method to ICalorimeterTool, which returns a pointer
to a new ICaloIndexer object.  This provides an interface for translating
cellIDs to indices.
@scott-snyder
Copy link
Contributor Author

To support performance improvements in cell/cluster reconstruction when all cells are present (i.e., with noise).

#define K4INTERFACE_ICALOINDEXER_H

#include <cstdint>
#include <memory>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
#include <memory>

Not used here


// Gaudi
#include "GaudiKernel/IAlgTool.h"

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
#include <memory>

/**
* @brief Return the IDs of the detector(s) that we index.
*/
virtual std::span<const int> detIDs() const = 0;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will this ID come from DD4hep's BitFieldCoder? If so it's an int64_t and then this span can not be constructed trivially.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It will actually be a uint64_t, I think: https://github.com/AIDASoft/DD4hep/blob/f211417464fd0d53f5aa0581db3f56425723395f/DDCore/include/DDSegmentation/BitFieldCoder.h#L25

Why not make this a dd4hep::CellID directly?

We did similar things in the past, e.g. in several PRs linked from this one: AIDASoft/DD4hep#1125


Is it a problem that the span cannot be constructed trivially?

Copy link
Member

@jmcarcell jmcarcell Mar 10, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But this is not cellID, cellID is set to uint64_t. And the point of the span is to have a view to a contiguous array, if the array is a different type then a new one has to be constructed, which would happen also for std::vector.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah OK. I didn't read carefully enough and thought this returns cell / volume Ids for the sensor. In that case, ignore my comment for this one, but there might be cases where one could consider going to dd4hep::CellID in other signatures above.

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