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

feat: Decouple SurfaceAccessor from source link implementations #3445

Merged
merged 18 commits into from
Aug 6, 2024

Conversation

ssdetlab
Copy link
Contributor

Making the TestSourceLink a template that takes the geometry type for compatibility with both gens of the geometry.

Adding the findSurface method to the Experimental::Detector, analogous to that of the TrackingGeometry.

@paulgessinger
Copy link
Member

Replying here to @andiwand's mention.

Yeah, this is a bit of a concern. We need to be careful how much we make Gen2 "production through the back door", before we've completed the needed review of the somewhat sprawling API surface.

At the same time, we should not just block all developments.

I would cautiously suggest favoring copying specific code for Gen2 and moving it into the Experimental namespace, with the understanding that this can and will be broken because we don't consider it stable by definition.

@ssdetlab
Copy link
Contributor Author

ssdetlab commented Aug 1, 2024

I get that we need to keep the Experimental somewhat separated from the stable codebase. But, on the other hand, it seems rather redundant to have a Test::Experimental and copying there almost everything to solve the issue that can be addressed in a couple of lines.

@github-actions github-actions bot added Component - Examples Affects the Examples module Track Finding labels Aug 2, 2024
@ssdetlab
Copy link
Contributor Author

ssdetlab commented Aug 2, 2024

After some discussions it was decided to decouple the surface accessors from the source link implementations. This way adding the overloads for the Gen2 is much less intrusive.

@ssdetlab ssdetlab changed the title feat: Making TestSourceLink compatible with both geometry Gens feat: Decouple SurfaceAccessor from source link implementations Aug 2, 2024
andiwand
andiwand previously approved these changes Aug 6, 2024
andiwand
andiwand previously approved these changes Aug 6, 2024
@andiwand andiwand added this to the next milestone Aug 6, 2024
Copy link

sonarcloud bot commented Aug 6, 2024

@kodiakhq kodiakhq bot merged commit ca69799 into acts-project:main Aug 6, 2024
46 checks passed
@github-actions github-actions bot removed the automerge label Aug 6, 2024
@acts-project-service acts-project-service added the Fails Athena tests This PR causes a failure in the Athena tests label Aug 6, 2024
@paulgessinger paulgessinger modified the milestones: next, v36.1.0 Aug 19, 2024
@ssdetlab ssdetlab deleted the detector-find-surface branch October 11, 2024 12:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Component - Core Affects the Core module Component - Examples Affects the Examples module Event Data Model Fails Athena tests This PR causes a failure in the Athena tests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants