Repository navigation
fix(event-display): convert jivexml vertex positions from cm to mm - #1067
Merged
Merged
Conversation
JiveXML writes lengths in cm, and the loader already multiplies hit positions and track polylines by 10 to get Phoenix's mm. The RVx x, y and z were passed through as they are, so every vertex was drawn at a tenth of its real distance from the origin. With the bundled JiveXML_336567_2327102923.xml the beam spot vertices end up around (-0.05, -0.09) mm instead of (-0.5, -0.9) mm, pile-up vertices are squeezed into a tenth of the luminous region, and a secondary vertex at r = 297 mm is drawn inside the pixel detector at r = 30 mm, away from the tracks that come out of it. Multiply x, y and z by 10 like the other positions, and add tests that check the conversion and that a vertex sits at the start of its linked track's polyline.
|
🚀 Preview deployed: http://phoenix-pr-1067.surge.sh Built from 18ad7a1. |
Member
|
Ouch. Nice catch - thanks! |
This branch was successfully deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
JiveXML writes lengths in cm.
JiveXMLLoaderalready multiplies hit positions and track polylines by 10 to get mm, butgetVerticespassed theRVxx, y and z through unchanged, so every vertex was drawn at a tenth of its real distance from the origin.You can see it with the bundled
JiveXML_336567_2327102923.xml:The last one is a secondary vertex at r = 297 mm that was being drawn inside the pixel detector at r = 30 mm, away from its tracks. As a cross check, in the same file the z at the beam line taken from the
Trackspolylines (mm) is 10.0 times the rawz0value (median over 158 tracks), so the raw JiveXML lengths really are cm.The fix multiplies x, y and z by 10, the same way the hits and polylines are converted.
Testing
Added two tests to
jivexml-loader.test.ts: one checks the cm to mm conversion, the other loads a track with a polyline plus a vertex linked to it and checks the vertex sits at the first polyline point.yarn test src/tests/loaders/jivexml-loadergives 2 failed, 3 passed (vertex x is 0.1 instead of 1).jest --maxWorkers=2): 48 suites, 442 tests passed.I didn't run the phoenix-ng tests or check it in the app UI.