Skip to content

Add a steering file for using the native EDM4hep algorithms#70

Open
jmcarcell wants to merge 79 commits into
mainfrom
add-native
Open

Add a steering file for using the native EDM4hep algorithms#70
jmcarcell wants to merge 79 commits into
mainfrom
add-native

Conversation

@jmcarcell
Copy link
Copy Markdown
Member

@jmcarcell jmcarcell commented Mar 28, 2025

BEGINRELEASENOTES

  • Modify the parameters in the tracking algorithms so that they can be used both in the original marlin reconstruction and using native EDM4hep algorithms
  • Add CLDReconstruction_iosvc.py, a steering file that uses IOSvc and the native algorithms when possible
  • Add a test to compare the collections that are obtained with the wrapper and with the native algorithms, to check that they are similar

ENDRELEASENOTES

Tested that the original marlin reconstruction gives the same results before and after since nothing should have changed for it, both with --truthTracking and without (up to some ID changes because I think there is non-deterministic ordering in Refit, but the tracks are the same).

Comment thread CLDConfig/py_utils.py Outdated
@jmcarcell jmcarcell marked this pull request as ready for review June 5, 2026 06:08
@jmcarcell
Copy link
Copy Markdown
Member Author

Planning on merging this soon @Zehvogel when CI passes and a few minor things are fixed

Copy link
Copy Markdown
Collaborator

@Zehvogel Zehvogel left a comment

Choose a reason for hiding this comment

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

Hi, thanks for this effort!

I am mostly very happy with the results. However, in some cases, like the conformal tracking, I think there is a bit too much chaos in the file. I have left some comments inline.



vxd_barrel_digitiser_args_marlin = toMarlinDict(vxd_barrel_digitiser_args)
vxd_barrel_digitiser_args_marlin["SubDetectorName"] = [vxd_barrel_digitiser_args["SubDetectorName"]]
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Is there a less ugly way to solve this?

ConformalTrackingSequence = [
MyConformalTracking,
ClonesAndSplitTracksFinder,
clones_and_split_tracks_finder,
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

any reason to rename this one in particular?

args = parser.parse_known_args()

# The keys are simply a name and are not passed to ConformalTracking
parameters = {
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Maybe this should be named steps instead to clarify what it contains?

from Configurables import MarlinProcessorWrapper
from k4FWCore.parseArgs import parser

from conformal_tracking_utils import configure_conformal_tracking_steps
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

where do these live? :o

Comment on lines +125 to +152
for name, param_dict in parameters.items():
marlin_collections = []
for i in range(len(param_dict["collections"])):
marlin_collections.append(param_dict["collections"][i])
if i < len(param_dict["collections"]) - 1:
marlin_collections[-1] += ","
marlin_parameters = []
for i, (k, v) in enumerate(param_dict["params"].items()):
marlin_parameters.extend([k, ":", f"{v};"])

marlin_flags = []
for i in range(len(param_dict["flags"])):
marlin_flags.append(param_dict["flags"][i])
if i < len(param_dict["flags"]) - 1:
marlin_flags[-1] += ","
marlin_functions = []
for i in range(len(param_dict["functions"])):
marlin_functions.append(param_dict["functions"][i])
if i < len(param_dict["functions"]) - 1:
marlin_functions[-1] += ","
current_step = [
f"[{name}]",
"@Collections", ":", *marlin_collections,
"@Parameters", ":", *marlin_parameters,
"@Flags", ":", *marlin_flags,
"@Functions", ":", *marlin_functions,
]
steps_marlin.extend(current_step)
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Can you put this into a utility function in e.g. py_utils?

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I know it is annoying, but is there any way we can make this file more readable again? :(

Comment thread test/CMakeLists.txt
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Any particular reason why some test names are in quotes and others are not?

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.

Migrate to 'Key4hep native' algorithms

3 participants