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

refactor: Make seeding config more understandable #2652

Merged
merged 34 commits into from
Dec 6, 2023

Conversation

LuisFelipeCoelho
Copy link
Member

@LuisFelipeCoelho LuisFelipeCoelho commented Nov 13, 2023

Re-writing mostly all comments from seeding configuration to make them more understandable.

@github-actions github-actions bot added Component - Core Affects the Core module Seeding labels Nov 13, 2023
Copy link

codecov bot commented Nov 13, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (04544d9) 49.48% compared to head (c3b546c) 49.48%.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #2652   +/-   ##
=======================================
  Coverage   49.48%   49.48%           
=======================================
  Files         474      474           
  Lines       27075    27075           
  Branches    12516    12516           
=======================================
  Hits        13399    13399           
  Misses       4762     4762           
  Partials     8914     8914           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@paulgessinger paulgessinger added this to the next milestone Nov 13, 2023
Core/include/Acts/Seeding/SeedConfirmationRangeConfig.hpp Outdated Show resolved Hide resolved
Core/include/Acts/Seeding/SeedConfirmationRangeConfig.hpp Outdated Show resolved Hide resolved
Core/include/Acts/Seeding/SeedFilterConfig.hpp Outdated Show resolved Hide resolved
Core/include/Acts/Seeding/SeedFilterConfig.hpp Outdated Show resolved Hide resolved
Core/include/Acts/Seeding/SeedFinderConfig.hpp Outdated Show resolved Hide resolved
Core/include/Acts/Seeding/SeedFinderConfig.hpp Outdated Show resolved Hide resolved
Core/include/Acts/Seeding/SeedFinderOrthogonalConfig.hpp Outdated Show resolved Hide resolved
LuisFelipeCoelho and others added 6 commits November 13, 2023 15:27
Co-authored-by: Alexander J. Pfleger <70842573+AJPfleger@users.noreply.github.com>
Co-authored-by: Alexander J. Pfleger <70842573+AJPfleger@users.noreply.github.com>
Co-authored-by: Alexander J. Pfleger <70842573+AJPfleger@users.noreply.github.com>
Co-authored-by: Alexander J. Pfleger <70842573+AJPfleger@users.noreply.github.com>
Co-authored-by: Alexander J. Pfleger <70842573+AJPfleger@users.noreply.github.com>
Co-authored-by: Alexander J. Pfleger <70842573+AJPfleger@users.noreply.github.com>
@paulgessinger
Copy link
Member

Do we have an idea why the physmon output changes?

@LuisFelipeCoelho
Copy link
Member Author

LuisFelipeCoelho commented Nov 13, 2023

Do we have an idea why the physmon output changes?

Maybe some default values of the orthogonal seeding were different from the default seeding, I will check if that is the case

@LuisFelipeCoelho
Copy link
Member Author

FYI: @CarloVarni

LuisFelipeCoelho and others added 2 commits November 29, 2023 13:31
Co-authored-by: Alexander J. Pfleger <70842573+AJPfleger@users.noreply.github.com>
Co-authored-by: Alexander J. Pfleger <70842573+AJPfleger@users.noreply.github.com>
AJPfleger
AJPfleger previously approved these changes Nov 29, 2023
@acts-project-service
Copy link
Collaborator

acts-project-service commented Dec 4, 2023

✅ Athena integration test results

✅ All tests successful

status job
🟢 run_unit_tests
🟢 run_ci_tests: ../athena/AtlasTest/CITest/test/ActsBenchmarkWithSpot.sh 8 100
🟢 run_ci_tests: ../athena/AtlasTest/CITest/test/ActsWorkflow.sh
🟢 run_ci_tests: ../athena/AtlasTest/CITest/test/ActsValidateAmbiguityResolution.sh
🟢 run_ci_tests: ../athena/AtlasTest/CITest/test/ActsValidateResolvedTracks.sh
🟢 run_ci_tests: ../athena/AtlasTest/CITest/test/ActsValidateTracks.sh
🟢 run_ci_tests: ../athena/AtlasTest/CITest/test/ActsValidateActsCoreSpacePoints.sh
🟢 run_ci_tests: ../athena/AtlasTest/CITest/test/ActsValidateActsSpacePoints.sh
🟢 run_ci_tests: ../athena/AtlasTest/CITest/test/ActsValidateSeeds.sh
🟢 run_ci_tests: ../athena/AtlasTest/CITest/test/ActsValidateOrthogonalSeeds.sh
🟢 run_ci_tests: ../athena/AtlasTest/CITest/test/ActsValidateClusters.sh
🟢 run_ci_tests: ../athena/AtlasTest/CITest/test/ActsPersistifyEDM.sh
🟢 run_ci_tests: ../athena/AtlasTest/CITest/test/ActsGSFRefitting.sh
🟢 run_ci_tests: ../athena/AtlasTest/CITest/test/ActsKfRefitting.sh
🟢 run_ci_tests: python3 ../athena/Tracking/Acts/ActsGeometry/test/ActsExtrapolationAlgTest.py
🟢 run_ci_tests: python3 ../athena/Tracking/Acts/ActsGeometry/test/ActsITkTest.py
🟢 run_workflow_tests_run4_mc
🟢 run_workflow_tests_run2_data
🟢 run_workflow_tests_run3_mc
🟢 run_workflow_tests_run3_data
🟢 run_art_test: test_data18_13TeV_1000evt
🟢 run_art_test: test_ttbarPU40_reco

@acts-project-service acts-project-service added the Breaks Athena build This PR breaks the Athena build label Dec 4, 2023
Copy link
Member

@paulgessinger paulgessinger left a comment

Choose a reason for hiding this comment

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

This seems to fail the Athena build. Can you either make it so it's backward compatible @LuisFelipeCoelho or we need to hold this until v32 next week.

Build failure is likely because the branch is not even with main. I'll update and retrigger

@paulgessinger
Copy link
Member

Ok it's green now, so good to go.

paulgessinger
paulgessinger previously approved these changes Dec 5, 2023
@paulgessinger paulgessinger added automerge ready Ready to merge, but not being merged yet and removed automerge Breaks Athena build This PR breaks the Athena build labels Dec 5, 2023
Co-authored-by: Paul Gessinger <hello@paulgessinger.com>
@andiwand andiwand added automerge and removed ready Ready to merge, but not being merged yet labels Dec 5, 2023
@kodiakhq kodiakhq bot merged commit fcbab62 into acts-project:main Dec 6, 2023
52 checks passed
@github-actions github-actions bot removed the automerge label Dec 6, 2023
@paulgessinger paulgessinger modified the milestones: next, v31.2.0 Dec 7, 2023
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 Seeding
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants