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

chore: Newline after #pragma once #3997

Merged

Conversation

andiwand
Copy link
Contributor

@andiwand andiwand commented Dec 18, 2024

Summary by CodeRabbit

  • New Features

    • Introduced multiple new classes for managing geometric surfaces, magnetic fields, and track density, enhancing the modeling capabilities within the ACTS project.
    • Added functionality for ONNX model inference, allowing integration of machine learning models into the framework.
    • Enhanced the Hough transform utilities with new methods for accessing unique hits and improved parameter handling.
  • Bug Fixes

    • Updated methods in the SPForSeed class to improve error correction for space points.
  • Documentation

    • Improved documentation for various classes and methods to enhance clarity and usability.
  • Chores

    • Added blank lines for improved code readability in several header files.

@andiwand andiwand added this to the next milestone Dec 18, 2024
@andiwand andiwand marked this pull request as ready for review December 18, 2024 14:26
Copy link

coderabbitai bot commented Dec 18, 2024

Walkthrough

The pull request introduces a comprehensive set of new header files and classes across multiple namespaces in the ACTS project, focusing on geometry, magnetic field management, surfaces, and utilities. The changes span core libraries and plugins, adding new interfaces, implementing utility classes, and enhancing existing functionality with more flexible and robust implementations.

Changes

File Path Change Summary
Core/include/Acts/Geometry/... Minor formatting changes and interface definitions
Core/include/Acts/MagneticField/... New magnetic field provider classes: ConstantBField, MultiRangeBField, NullBField, and utility functions
Core/include/Acts/Surfaces/... New surface bound and surface classes like DiscTrapezoidBounds, LineSurface, PerigeeSurface, SurfaceArray
Core/include/Acts/Utilities/... Added utility classes for binning and quadratic equation solving
Plugins/... New plugin implementations for DD4hep, Legacy seeding, and ONNX runtime

Sequence Diagram

sequenceDiagram
    participant MagneticFieldProvider
    participant ConstantBField
    participant MultiRangeBField
    participant NullBField

    MagneticFieldProvider->>ConstantBField: Implement constant field
    MagneticFieldProvider->>MultiRangeBField: Support multiple field ranges
    MagneticFieldProvider->>NullBField: Provide zero field implementation
Loading

Possibly related PRs

Poem

In realms of code, where bytes do dance, 🕺
A new PR springs, with geometric stance 🌐
Magnetic fields and surfaces bright 🌟
Yoda's wisdom, compiled just right! 🧘‍♂️
Hmmm, elegant code, it has become! 🚀


Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media?

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (Invoked using PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai generate docstrings to generate docstrings for this PR. (Beta)
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

@github-actions github-actions bot added Component - Core Affects the Core module Component - Plugins Affects one or more Plugins Vertexing Seeding labels Dec 18, 2024
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

🧹 Nitpick comments (5)
Core/include/Acts/Surfaces/LineSurface.hpp (1)

10-10: Newline after #pragma once, add you must.
Consistent formatting, it will promote.

Apply this diff:

 #pragma once
+
Core/include/Acts/Surfaces/SurfaceArray.hpp (1)

10-10: Newline after #pragma once, place you must.
Smooth reading, it shall ensure.

Apply this diff:

 #pragma once
+
Plugins/Onnx/include/Acts/Plugins/Onnx/OnnxRuntimeBase.hpp (1)

10-10: A blank line after #pragma once, add you should.
Orderly code style, it creates.

Apply this diff:

 #pragma once
+
Core/include/Acts/Utilities/ThrowAssert.hpp (1)

10-10: After #pragma once, an extra newline you must insert.
Uniform code formatting we achieve, mm?

Apply this diff:

 #pragma once
+
Core/include/Acts/Seeding/HoughTransformUtils.hpp (1)

Line range hint 160-168: Additional widthPar parameter, powerful extension it is.
Expanding flexibility in binning logic, the extra function provides. Careful to document usage in Doxygen you should, so other Jedi on the project clarity have.

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 9269562 and 274375c.

📒 Files selected for processing (27)
  • Core/include/Acts/Geometry/IConfinedTrackingVolumeBuilder.hpp (1 hunks)
  • Core/include/Acts/Geometry/ITrackingGeometryBuilder.hpp (1 hunks)
  • Core/include/Acts/Geometry/ProtoLayer.hpp (1 hunks)
  • Core/include/Acts/MagneticField/BFieldMapUtils.hpp (1 hunks)
  • Core/include/Acts/MagneticField/ConstantBField.hpp (1 hunks)
  • Core/include/Acts/MagneticField/MultiRangeBField.hpp (1 hunks)
  • Core/include/Acts/MagneticField/NullBField.hpp (1 hunks)
  • Core/include/Acts/Seeding/HoughTransformUtils.hpp (1 hunks)
  • Core/include/Acts/Surfaces/DiscTrapezoidBounds.hpp (1 hunks)
  • Core/include/Acts/Surfaces/LineSurface.hpp (1 hunks)
  • Core/include/Acts/Surfaces/PerigeeSurface.hpp (1 hunks)
  • Core/include/Acts/Surfaces/SurfaceArray.hpp (1 hunks)
  • Core/include/Acts/Surfaces/SurfaceBounds.hpp (1 hunks)
  • Core/include/Acts/Surfaces/TrapezoidBounds.hpp (1 hunks)
  • Core/include/Acts/Utilities/BinnedArray.hpp (1 hunks)
  • Core/include/Acts/Utilities/BinnedArrayXD.hpp (1 hunks)
  • Core/include/Acts/Utilities/BinningType.hpp (1 hunks)
  • Core/include/Acts/Utilities/ThrowAssert.hpp (1 hunks)
  • Core/include/Acts/Utilities/detail/RealQuadraticEquation.hpp (1 hunks)
  • Core/include/Acts/Vertexing/AdaptiveGridTrackDensity.hpp (1 hunks)
  • Core/include/Acts/Vertexing/GaussianGridTrackDensity.hpp (1 hunks)
  • Plugins/DD4hep/include/Acts/Plugins/DD4hep/DD4hepVolumeBuilder.hpp (1 hunks)
  • Plugins/Legacy/include/Acts/Seeding/AtlasSeedFinder.hpp (1 hunks)
  • Plugins/Legacy/include/Acts/Seeding/LegacyInternalSeed.hpp (1 hunks)
  • Plugins/Legacy/include/Acts/Seeding/LegacySeed.hpp (1 hunks)
  • Plugins/Legacy/include/Acts/Seeding/SPForSeed.hpp (1 hunks)
  • Plugins/Onnx/include/Acts/Plugins/Onnx/OnnxRuntimeBase.hpp (1 hunks)
✅ Files skipped from review due to trivial changes (3)
  • Core/include/Acts/Geometry/ITrackingGeometryBuilder.hpp
  • Core/include/Acts/Geometry/IConfinedTrackingVolumeBuilder.hpp
  • Core/include/Acts/Utilities/BinningType.hpp
🧰 Additional context used
📓 Learnings (1)
Core/include/Acts/MagneticField/BFieldMapUtils.hpp (1)
Learnt from: AJPfleger
PR: acts-project/acts#3615
File: Core/src/MagneticField/BFieldMapUtils.cpp:214-231
Timestamp: 2024-12-02T10:20:44.377Z
Learning: In the C++ file `Core/src/MagneticField/BFieldMapUtils.cpp`, when passing methods to constructors in functions like `fieldMapRZ` and `solenoidFieldMap`, prefer to define transforms in each function separately for clarity, even if the methods are similar.
🔇 Additional comments (21)
Core/include/Acts/Utilities/BinnedArray.hpp (1)

14-14: Hmmm, spacing aligned properly after pragma once, it is.
A uniform style throughout the codebase, we must maintain.

Core/include/Acts/Utilities/BinnedArrayXD.hpp (1)

14-14: Newline after pragma once, good practice it is.
Approved, this alignment is.

Plugins/Legacy/include/Acts/Seeding/AtlasSeedFinder.hpp (1)

14-14: Hmmm, maintain clarity, this does.
A newline after "#pragma once" ensures neatness, yes. Enhances readability, it does.

Plugins/Legacy/include/Acts/Seeding/LegacySeed.hpp (1)

14-14: Helpful, this small change is.
Preserves formatting consistency, it does. Newline after pragma once, a fine practice it is.

Plugins/Legacy/include/Acts/Seeding/LegacyInternalSeed.hpp (1)

14-14: Aligned with standard conventions, this is.
Yes, keep the codebase tidy, we must.

Plugins/Legacy/include/Acts/Seeding/SPForSeed.hpp (1)

14-14: Commendable, the spacing after pragma once is.
Yes, fosters cleanliness in the structure, hmm.

Core/include/Acts/MagneticField/MultiRangeBField.hpp (1)

10-10: A mere newline after #pragma once, hmm. An improvement, it is.
Align with the style guidelines, this does. No issues I see. Approve the change, I do.

Core/include/Acts/Surfaces/TrapezoidBounds.hpp (1)

10-10: New line, yes. Good for readability, it is.
Enhance clarity, such a small change does.

Core/include/Acts/Surfaces/DiscTrapezoidBounds.hpp (1)

10-10: After #pragma once, a space you add.
Consistent with a neat code layout, this is. No concerns have I.

Core/include/Acts/Vertexing/AdaptiveGridTrackDensity.hpp (1)

10-10: A new line you inserted, wise choice.
Maintain code style, such changes will. Continue, you must.

Core/include/Acts/Utilities/detail/RealQuadraticEquation.hpp (1)

13-14: Approve this change, I do!

Consistent with coding style, the newline after #pragma once is. Improved readability, it has!

Core/include/Acts/MagneticField/NullBField.hpp (1)

9-10: Approve this change too, I must!

A newline after #pragma once, added it has been. Good separation between preprocessor directives and includes, this provides!

Core/include/Acts/Surfaces/SurfaceBounds.hpp (1)

9-10: Approve this change as well, we shall!

The Force is strong with this one! A newline after #pragma once, properly placed it is.

Core/include/Acts/MagneticField/ConstantBField.hpp (1)

10-10: Approve newline addition after pragma once, hmm yes.

Improved readability, this change has. Consistent with coding style guidelines, it is.

Core/include/Acts/Surfaces/PerigeeSurface.hpp (1)

10-10: Approve newline addition after pragma once, I do.

Clarity to the Force, this newline brings. Good separation between header guard and includes, it provides.

Plugins/DD4hep/include/Acts/Plugins/DD4hep/DD4hepVolumeBuilder.hpp (1)

10-10: Approve newline addition after pragma once, we must.

Balance to the code structure, this newline brings. Consistent with the other files, it now is.

Core/include/Acts/Geometry/ProtoLayer.hpp (1)

9-10: Approve this change, I do!

Improved readability between pragma directive and includes, this newline brings. Consistent with coding style guidelines, it is.

Core/include/Acts/MagneticField/BFieldMapUtils.hpp (1)

9-10: Approve this change, I do!

A clear separation between pragma and includes, this newline creates. Consistent with other files in the codebase, it remains.

Core/include/Acts/Vertexing/GaussianGridTrackDensity.hpp (1)

9-10: Approve this change, I do!

Better readability between pragma and includes, this newline provides. Maintains consistency across the codebase, it does.

Core/include/Acts/Seeding/HoughTransformUtils.hpp (2)

12-12: Newline after #pragma once, good practice it is.
A neat separation from the includes you have provided, hmmm. Clarity and consistency, improved they are.


Line range hint 73-73: New getter method for hits, reveal its potential it does.
Yes, returns a span of identifies, mm? Ensure usage across codebase is correct and thoroughly tested, you should. Save time in debugging, it will.

Copy link

📊: Physics performance monitoring for 274375c

Full contents

physmon summary

Copy link
Member

@benjaminhuth benjaminhuth left a comment

Choose a reason for hiding this comment

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

Looks good to me. Should there be a automated check?

@kodiakhq kodiakhq bot merged commit cdaa905 into acts-project:main Dec 18, 2024
48 checks passed
@andiwand andiwand deleted the chore-newline-after-pragma-once branch December 18, 2024 18:25
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 Component - Plugins Affects one or more Plugins Seeding Vertexing
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants