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: Remove redundant check in MultiWireStructureBuilder #2824

Merged
merged 12 commits into from
Dec 15, 2023

Conversation

dimitra97
Copy link
Contributor

@dimitra97 dimitra97 commented Dec 13, 2023

I removed some lines that check the dimensions of the trapezoid bounds since it is already checked in the volume structure builder that is called in the construct method of the MultiWireStructureBuilder.
Also, it does not allow for passing six parameters and using the six-parameter constructor of the TrapezoidVolumeBounds
I need to get rid of this in order to be able to pass all the bound values (six values) when constructing the bounds of MDTS in athena:

std::unique_ptr<Acts::TrapezoidVolumeBounds> mdtBounds = std::make_unique<Acts::TrapezoidVolumeBounds>(parameters.shortHalfX, parameters.longHalfX, parameters.halfY, parameters.halfHeight); mlCfg.mlBounds= mdtBounds->values();

dimitra97 and others added 7 commits December 7, 2023 12:19
An indexed surfaces multilayer navigation

Remove unused variable

Remove unused variable

fix type conversions for the number of bins

changes on the mockupbuilder header file and on the unit test

Update Core/include/Acts/Navigation/MultiWireLayerUpdators.hpp

Co-authored-by: Andreas Stefl <stefl.andreas@gmail.com>

Change on test mockup builder script

Multi Wire structure with the interface

Changes on the multiwire structure builder

Place the files that create the mockup geometry in another folder

Change the location of the gdml file

An indexed surfaces multilayer navigation

Remove unused variable

fix type conversions for the number of bins

Multi Wire structure with the interface

Delete MultiWireLayerUpdators.hpp

Update CMakeLists.txt

revert some files

trying for Indexed Surfaces Generator

Indexed Surfaces generator update

fix

LayerStructure builder fix

fix

Delete MuonChamber.gdml

revert layer strucutre builder from upstream

reslove conflict

Multi Layer Builder

cmake file

Place the files that create the mockup geometry in another folder

Change the location of the gdml file

An indexed surfaces multilayer navigation

Remove unused variable

Remove unused variable

fix type conversions for the number of bins

Remove some actsvg includes not needed now

change the path for the gdml file

changes on the mockupbuilder header file and on the unit test

Update Core/include/Acts/Navigation/MultiWireLayerUpdators.hpp

Co-authored-by: Andreas Stefl <stefl.andreas@gmail.com>

Change on test mockup builder script

Multi Wire structure with the interface

Changes on the multiwire structure builder

Place the files that create the mockup geometry in another folder

Change the location of the gdml file

An indexed surfaces multilayer navigation

Remove unused variable

fix type conversions for the number of bins

Remove some actsvg includes not needed now

change the path for the gdml file

Multi Wire structure with the interface

Delete MultiWireLayerUpdators.hpp

Delete IndexedSurfacesNavigationTests.cpp

Update CMakeLists.txt

revert some files

trying for Indexed Surfaces Generator

Indexed Surfaces generator update

fix

LayerStructure builder fix

fix

Delete MuonChamber.gdml

revert layer strucutre builder from upstream

cmake file

Update MultiWireStructureBuilder.hpp

Update MultiWireStructureBuilder.hpp

Update MultiWireStructureBuilder.cpp

Place the files that create the mockup geometry in another folder

Change the location of the gdml file

An indexed surfaces multilayer navigation

Remove unused variable

Remove unused variable

fix type conversions for the number of bins

Remove some actsvg includes not needed now

change the path for the gdml file

changes on the mockupbuilder header file and on the unit test

Update Core/include/Acts/Navigation/MultiWireLayerUpdators.hpp

Co-authored-by: Andreas Stefl <stefl.andreas@gmail.com>

Change on test mockup builder script

Multi Wire structure with the interface

Changes on the multiwire structure builder

Place the files that create the mockup geometry in another folder

Change the location of the gdml file

An indexed surfaces multilayer navigation

Remove unused variable

fix type conversions for the number of bins

Remove some actsvg includes not needed now

change the path for the gdml file

Multi Wire structure with the interface

Delete MultiWireLayerUpdators.hpp

Delete IndexedSurfacesNavigationTests.cpp

Update CMakeLists.txt

revert some files

trying for Indexed Surfaces Generator

Indexed Surfaces generator update

LayerStructure builder fix

fix

Delete MuonChamber.gdml

revert gdml from upstream

revert layer strucutre builder from upstream

fix conflicts and some optimizations

conflicts and format

revert some files

revert some files

new updator

fix

license issue

 fix
@github-actions github-actions bot added the Component - Core Affects the Core module label Dec 13, 2023
Copy link

codecov bot commented Dec 13, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (69290be) 48.85% compared to head (3da2d79) 48.85%.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #2824   +/-   ##
=======================================
  Coverage   48.85%   48.85%           
=======================================
  Files         486      486           
  Lines       28178    28175    -3     
  Branches    13292    13290    -2     
=======================================
  Hits        13766    13766           
+ Misses       4796     4794    -2     
+ Partials     9616     9615    -1     

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

Copy link
Contributor

@AJPfleger AJPfleger left a comment

Choose a reason for hiding this comment

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

lgtm. I think we don't need the check here, because the TrapezoidVolumeBounds will not compile, if the the size is not 4/5/6.

@paulgessinger paulgessinger changed the title refactor: Mwbuilder minor refactor: Remove redundant check in MultiWireStructureBuilder Dec 15, 2023
@andiwand andiwand added this to the next milestone Dec 15, 2023
@kodiakhq kodiakhq bot merged commit c60763d into acts-project:main Dec 15, 2023
52 checks passed
@acts-project-service
Copy link
Collaborator

✅ Athena integration test results [c60763d]

✅ All tests successful

status job report
🟢 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_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

@paulgessinger paulgessinger modified the milestones: next, v32.0.0 Jan 19, 2024
LaraCalic pushed a commit to LaraCalic/acts that referenced this pull request Feb 10, 2024
…roject#2824)

I removed some lines that check the dimensions of the trapezoid bounds since it is already checked in the volume structure builder that is called in the `construct` method of the `MultiWireStructureBuilder. `
Also, it does not allow for passing six parameters and using the six-parameter constructor of the `TrapezoidVolumeBounds`
I need to get rid of this in order to be able to pass all the bound values (six values) when constructing the bounds of MDTS in athena: 

` std::unique_ptr<Acts::TrapezoidVolumeBounds> mdtBounds = std::make_unique<Acts::TrapezoidVolumeBounds>(parameters.shortHalfX, parameters.longHalfX, parameters.halfY, parameters.halfHeight);
       mlCfg.mlBounds= mdtBounds->values();
                     `
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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants