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

feat: Add INFO printouts about the layers & volumes #1773

Merged
merged 8 commits into from
Jan 30, 2023

Conversation

tboldagh
Copy link
Contributor

@tboldagh tboldagh commented Jan 3, 2023

This PR adds permanent (to be discussed) messages about the geometry volumes & layers numbering.
It seems to be the only robust way to get this information.
The implementation is a variant of what @gagnonlg suggested.

I can imagine an additional parameter steering if this messages occur.
Tagging @asalzburger @gagnonlg

@tboldagh tboldagh added Component - Core Affects the Core module Improvement Changes to an existing feature Impact - Minor Nuissance bug and/or affects only a single module Feature Development to integrate a new feature Needs Decision Needs decision or further information labels Jan 3, 2023
@tboldagh tboldagh added this to the next milestone Jan 3, 2023
@codecov
Copy link

codecov bot commented Jan 3, 2023

Codecov Report

Merging #1773 (fd8d1ae) into main (d20321a) will increase coverage by 0.00%.
The diff coverage is 46.66%.

@@           Coverage Diff           @@
##             main    #1773   +/-   ##
=======================================
  Coverage   49.54%   49.55%           
=======================================
  Files         407      407           
  Lines       22618    22625    +7     
  Branches    10318    10321    +3     
=======================================
+ Hits        11207    11211    +4     
  Misses       4230     4230           
- Partials     7181     7184    +3     
Impacted Files Coverage Δ
Core/include/Acts/Geometry/Layer.hpp 100.00% <ø> (ø)
Core/include/Acts/Geometry/TrackingGeometry.hpp 100.00% <ø> (ø)
Core/include/Acts/Geometry/TrackingVolume.hpp 70.73% <ø> (ø)
Core/include/Acts/Propagator/Navigator.hpp 54.36% <0.00%> (ø)
Core/src/Geometry/Layer.cpp 57.03% <0.00%> (-0.43%) ⬇️
Core/src/Geometry/TrackingGeometry.cpp 47.36% <50.00%> (ø)
Core/src/Geometry/TrackingVolume.cpp 46.28% <50.00%> (+0.48%) ⬆️
Core/src/Geometry/TrackingGeometryBuilder.cpp 47.61% <100.00%> (-4.77%) ⬇️
.../include/Acts/Geometry/TrackingGeometryBuilder.hpp 100.00% <0.00%> (ø)

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

@github-actions
Copy link

github-actions bot commented Jan 3, 2023

📊 Physics performance monitoring for fd8d1ae

Full report
Seeding: seeded, truth estimated, orthogonal
CKF: seeded, truth smeared, truth estimated, orthogonal
IVF: seeded, truth smeared, truth estimated, orthogonal
Ambiguity resolution: seeded, orthogonal
Truth tracking
Truth tracking (GSF)

Vertexing

Vertexing vs. mu
IVF seeded

IVF truth_smeared

IVF truth_estimated

IVF orthogonal

Seeding

Seeding seeded

Seeding truth_estimated

Seeding orthogonal

CKF

CKF seeded

CKF truth_smeared

CKF truth_estimated

CKF orthogonal

Ambiguity resolution

seeded

Truth tracking (Kalman Filter)

Truth tracking

Truth tracking (GSF)

Truth tracking

@tboldagh tboldagh marked this pull request as ready for review January 11, 2023 12:08
@paulgessinger
Copy link
Member

The pattern seems fine to me. If you can get rid of the LoggerWrappers, I'm happy!

@paulgessinger
Copy link
Member

Unit tests currently failing, because the logger seems to have a configuration problem.

@paulgessinger paulgessinger added 👷‍♀️ User Action Needed and removed Needs Decision Needs decision or further information labels Jan 17, 2023
@tboldagh
Copy link
Contributor Author

The test seem to fail in quite obscure way on licence checks and include guards but no hint is given as to where and what is incorrect. Tagging @paulgessinger

@paulgessinger
Copy link
Member

I think this is a failure on GitHub's side

@tboldagh
Copy link
Contributor Author

Is there anything that I can do to make it work?

@paulgessinger
Copy link
Member

#1799 This should fix it I believe, @tboldagh.

@tboldagh
Copy link
Contributor Author

@paulgessinger

#1799 This should fix it I believe, @tboldagh.

There seems to be still some issue. I have made last push after the #1799 was merged

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.

Just the approval missing I think.

@kodiakhq kodiakhq bot merged commit bdf2378 into acts-project:main Jan 30, 2023
@paulgessinger paulgessinger modified the milestones: next, v23.1.0 Feb 2, 2023
@gagnonlg
Copy link
Contributor

gagnonlg commented Apr 7, 2023

@tboldagh @paulgessinger how do you get this feature to work with the python-based workflow? I (perhaps naively) tried doing something like

    acts.logging.setLevel(acts.logging.DEBUG)
    detector, trackingGeometry, decorators = acts.examples.GenericDetector.create()

But nothing was printed

@paulgessinger
Copy link
Member

@gagnonlg the setLevel call does not set a global level for the other components. I believe GenericDetector has log levels in its config that you can set (you might be able to do this as kwargs to the create() call.

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 Feature Development to integrate a new feature Impact - Minor Nuissance bug and/or affects only a single module Improvement Changes to an existing feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants