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

fix: Generic Detector negative strip endcap rotation #2358

Merged

Conversation

paulgessinger
Copy link
Member

Sensors were flipped along local z, meaning they are the wrong way around.

Sensors were flipped along local z, meaning they are the wrong way
around.
@paulgessinger paulgessinger added this to the next milestone Aug 9, 2023
@github-actions github-actions bot added the Component - Examples Affects the Examples module label Aug 9, 2023
niermann999
niermann999 previously approved these changes Aug 9, 2023
@codecov
Copy link

codecov bot commented Aug 9, 2023

Codecov Report

Merging #2358 (14900ec) into main (a872842) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##             main    #2358   +/-   ##
=======================================
  Coverage   49.69%   49.69%           
=======================================
  Files         454      454           
  Lines       25809    25809           
  Branches    11852    11852           
=======================================
  Hits        12825    12825           
  Misses       4581     4581           
  Partials     8403     8403           

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@paulgessinger
Copy link
Member Author

As expected, physmon is happy but a lot of hashes change.

@andiwand
Copy link
Contributor

I fear we also need some updates in test_digitization_example - there are hardcoded volumes and layers

@paulgessinger
Copy link
Member Author

@andiwand That's confusing actually. Does this change the module numbers maybe?

@niermann999
Copy link
Contributor

@andiwand That's confusing actually. Does this change the module numbers maybe?

The modules will be placed at different phi values in the same ring, I think

@andiwand
Copy link
Contributor

If I understand this correctly the geoid should be the same no? because we only rotate/flip the modules

but we might hit them differently because of the geometry change which might cause this test to fail

@paulgessinger
Copy link
Member Author

Does this rotate the module about the center of the disk and not the module center? Or why would the phi values changes?

@niermann999
Copy link
Contributor

Nothing is rotated compared to the previous version. The translation was such that a correctly rotated modules was placed on the wrong side of the disc (mirrored at the origin compared to the positive endcap) thus effectively flipping it. If this becomes a problem with the module numbers, then we have to solve it indeed by changing the rotation instead of the translation and flip the modules instead of placing them differently, but that would make the code less intuitive, I thought

@paulgessinger
Copy link
Member Author

Why does this keep flip-flopping between failed hashes and conflicts in the hash file 😠

@paulgessinger
Copy link
Member Author

/run-experiment atlas

@kodiakhq kodiakhq bot merged commit 0c87313 into acts-project:main Sep 1, 2023
57 checks passed
@github-actions github-actions bot removed the automerge label Sep 1, 2023
@acts-project-service acts-project-service added the Fails Athena tests This PR causes a failure in the Athena tests label Sep 1, 2023
@paulgessinger paulgessinger modified the milestones: next, v29.1.0 Sep 7, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Changes Performance Component - Examples Affects the Examples module 👷‍♀️ User Action Needed 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.

None yet

4 participants