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: and refactor geometric digitization #2630

Merged
merged 12 commits into from
Nov 13, 2023

Conversation

benjaminhuth
Copy link
Member

@benjaminhuth benjaminhuth commented Nov 4, 2023

  • fix: activation was only 2D path, but must be 3D path. Otherwise, an upright track would get 0 activation...
  • refactor: rename Channelizer to Segmentizer, add Channelizer that ties together the things
  • added unit test

@benjaminhuth benjaminhuth added this to the next milestone Nov 4, 2023
@github-actions github-actions bot added Component - Fatras Affects the Fatras module Component - Examples Affects the Examples module Event Data Model labels Nov 4, 2023
Copy link

codecov bot commented Nov 4, 2023

Codecov Report

Merging #2630 (e2d36bd) into main (85a0fea) will not change coverage.
The diff coverage is n/a.

❗ Current head e2d36bd differs from pull request most recent head 7acfdf9. Consider uploading reports for the commit 7acfdf9 to get more accurate results

@@           Coverage Diff           @@
##             main    #2630   +/-   ##
=======================================
  Coverage   49.71%   49.71%           
=======================================
  Files         474      474           
  Lines       26859    26859           
  Branches    12362    12362           
=======================================
  Hits        13352    13352           
  Misses       4706     4706           
  Partials     8801     8801           

📣 Codecov offers a browser extension for seamless coverage viewing on GitHub. Try it in Chrome or Firefox today!

@asalzburger
Copy link
Contributor

Unit test is failing - is that expected ? Do we need to update the test?

@benjaminhuth
Copy link
Member Author

Hmm no is not expected. They should run through. I will check after the workshop

@AJPfleger
Copy link
Contributor

Could you also adapt the tests to what I did in #2632 ? :)

I didn't touch Tests/UnitTests/Fatras/Digitization/ChannelizerTests.cpp in my PR to avoid conflicts on your side.
Also the file Tests/UnitTests/Fatras/Digitization/SegmentizerTests.cpp will be new for me.

asalzburger
asalzburger previously approved these changes Nov 10, 2023
Copy link
Contributor

@asalzburger asalzburger left a comment

Choose a reason for hiding this comment

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

Makes sense, let's fix the UnitTests and get that in.

@benjaminhuth
Copy link
Member Author

@asalzburger The PR should now be ready, can you approve?

benjaminhuth and others added 3 commits November 12, 2023 23:14
Co-authored-by: Alexander J. Pfleger <70842573+AJPfleger@users.noreply.github.com>
Co-authored-by: Alexander J. Pfleger <70842573+AJPfleger@users.noreply.github.com>
@kodiakhq kodiakhq bot merged commit a01f82a into acts-project:main Nov 13, 2023
54 checks passed
@acts-project-service
Copy link
Collaborator

acts-project-service commented Nov 13, 2023

🔴 Athena integration test results

Build job with this PR failed!

Please investigate the build job for the pipeline!

@acts-project-service acts-project-service added the Breaks Athena build This PR breaks the Athena build label Nov 13, 2023
@paulgessinger paulgessinger modified the milestones: next, v31.0.0 Nov 15, 2023
@paulgessinger paulgessinger removed the Breaks Athena build This PR breaks the Athena build label Nov 16, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants