Skip to content

feat(base): Introduce FairDetector::GetRootManager#1568

Merged
dennisklein merged 1 commit intoFairRootGroup:devfrom
ChristianTackeGSI:pr/det_frm
Jul 2, 2024
Merged

feat(base): Introduce FairDetector::GetRootManager#1568
dennisklein merged 1 commit intoFairRootGroup:devfrom
ChristianTackeGSI:pr/det_frm

Conversation

@ChristianTackeGSI
Copy link
Member

Especially classes deriving from FairDetector and
implementing the Register member function need access to FairrootManager. Give them a proper getter.


Checklist:

@coderabbitai
Copy link

coderabbitai bot commented Jun 24, 2024

Walkthrough

Walkthrough

The changes primarily involve refactoring the registration methodology within various classes in the codebase to use GetRootManager() instead of directly accessing the singleton FairRootManager::Instance(). This update has been applied across multiple examples, templates, and core classes, likely to improve code modularity and maintainability.

Changes

File/Directory Change Summary
examples/MQ/pixelDetector/src/Pixel.cxx Changed method call from FairRootManager::Instance()->Register to GetRootManager().Register
examples/advanced/Tutorial3/simulation/FairTestDetector.cxx Refactored to use GetRootManager().Register instead of FairRootManager::Instance()->Register
examples/advanced/propagator/src/FairTutPropDet.cxx Updated Register method to use GetRootManager() and updated the copyright year to 2024
examples/simulation/Tutorial1/src/FairFastSimExample.cxx Modified method call to use GetRootManager().Register instead of FairRootManager::Instance()->Register
examples/simulation/Tutorial1/src/FairFastSimExample2.cxx Changed registration method call to use GetRootManager().Register
examples/simulation/Tutorial1/src/FairTutorialDet1.cxx Updated method call for registration to GetRootManager().Register
examples/simulation/Tutorial4/src/mc/FairTutorialDet4.cxx Switched registration method to use GetRootManager().Register
examples/simulation/rutherford/src/FairRutherford.cxx Modified FairRutherford to use GetRootManager() for registration
fairroot/base/sim/FairDetector.h Added #include <cassert>, method for setting/getting RootManager, and updated comments
fairroot/base/sim/FairMCApplication.cxx Updated RegisterOutput method to set RootManager for the detector
fairroot/base/steer/FairRootManager.h Added deprecation warning for FairRootManager::Instance() and suggested using specific getters instead
templates/NewDetector_root_containers/NewDetector.cxx Changed method calls for registration to use GetRootManager().Register based on simulation type
templates/NewDetector_stl_containers/NewDetector.cxx Updated Register method call to use GetRootManager().RegisterAny instead of FairRootManager::Instance()->RegisterAny
templates/project_root_containers/MyProjGenerators/Pythia8Generator.cxx Updated copyright year to 2024 and removed commented-out code
templates/project_root_containers/NewDetector/NewDetector.cxx Refactored Register method calls to use GetRootManager().Register and GetRootManager().RegisterAny based on condition related to gMC->IsMT()
templates/project_stl_containers/MyProjGenerators/Pythia8Generator.cxx Updated copyright year to 2024 and commented out code snippet related to creating a separate container using FairRootManager
templates/project_stl_containers/NewDetector/NewDetector.cxx Changed registration method call in Register() method to use GetRootManager().RegisterAny instead of FairRootManager::Instance()->RegisterAny

Sequence Diagram(s)

sequenceDiagram
    participant DetectorClass
    participant FairRootManager
    participant RootManager
    
    DetectorClass ->> RootManager: GetRootManager()
    activate RootManager
    RootManager ->> DetectorClass: Return RootManager instance
    deactivate RootManager
    
    DetectorClass ->> RootManager: Register("CollectionName", "CollectionType", collection, kTRUE)
Loading

Recent review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between 8e3a279 and ec56c6f.

Files selected for processing (17)
  • examples/MQ/pixelDetector/src/Pixel.cxx (2 hunks)
  • examples/advanced/Tutorial3/simulation/FairTestDetector.cxx (2 hunks)
  • examples/advanced/propagator/src/FairTutPropDet.cxx (2 hunks)
  • examples/simulation/Tutorial1/src/FairFastSimExample.cxx (2 hunks)
  • examples/simulation/Tutorial1/src/FairFastSimExample2.cxx (2 hunks)
  • examples/simulation/Tutorial1/src/FairTutorialDet1.cxx (2 hunks)
  • examples/simulation/Tutorial4/src/mc/FairTutorialDet4.cxx (2 hunks)
  • examples/simulation/rutherford/src/FairRutherford.cxx (2 hunks)
  • fairroot/base/sim/FairDetector.h (6 hunks)
  • fairroot/base/sim/FairMCApplication.cxx (1 hunks)
  • fairroot/base/steer/FairRootManager.h (2 hunks)
  • templates/NewDetector_root_containers/NewDetector.cxx (1 hunks)
  • templates/NewDetector_stl_containers/NewDetector.cxx (1 hunks)
  • templates/project_root_containers/MyProjGenerators/Pythia8Generator.cxx (2 hunks)
  • templates/project_root_containers/NewDetector/NewDetector.cxx (1 hunks)
  • templates/project_stl_containers/MyProjGenerators/Pythia8Generator.cxx (2 hunks)
  • templates/project_stl_containers/NewDetector/NewDetector.cxx (1 hunks)
Files skipped from review as they are similar to previous changes (16)
  • examples/MQ/pixelDetector/src/Pixel.cxx
  • examples/advanced/Tutorial3/simulation/FairTestDetector.cxx
  • examples/advanced/propagator/src/FairTutPropDet.cxx
  • examples/simulation/Tutorial1/src/FairFastSimExample.cxx
  • examples/simulation/Tutorial1/src/FairFastSimExample2.cxx
  • examples/simulation/Tutorial1/src/FairTutorialDet1.cxx
  • examples/simulation/Tutorial4/src/mc/FairTutorialDet4.cxx
  • examples/simulation/rutherford/src/FairRutherford.cxx
  • fairroot/base/sim/FairDetector.h
  • fairroot/base/sim/FairMCApplication.cxx
  • templates/NewDetector_root_containers/NewDetector.cxx
  • templates/NewDetector_stl_containers/NewDetector.cxx
  • templates/project_root_containers/MyProjGenerators/Pythia8Generator.cxx
  • templates/project_root_containers/NewDetector/NewDetector.cxx
  • templates/project_stl_containers/MyProjGenerators/Pythia8Generator.cxx
  • templates/project_stl_containers/NewDetector/NewDetector.cxx

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>.
    • 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 generate interesting stats about this repository and render them as a table.
    • @coderabbitai show all the console.log statements in this repository.
    • @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 as 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 resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Additionally, you can add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.

CodeRabbit Configration 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.

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: 1

karabowi
karabowi previously approved these changes Jun 25, 2024
Especially classes deriving from FairDetector and
implementing the Register member function need access to
FairrootManager. Give them a proper getter.
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

Outside diff range and nitpick comments (1)
fairroot/base/steer/FairRootManager.h (1)

139-149: Document deprecation warning clearly for Instance() method.

The deprecation warning in the documentation of the Instance() method is a good practice as it guides the future development towards more maintainable code using specific getters. However, it would be beneficial to include a timeline or condition under which this method will be deprecated to provide clearer guidance to developers.

@dennisklein dennisklein merged commit c4476dd into FairRootGroup:dev Jul 2, 2024
@ChristianTackeGSI ChristianTackeGSI deleted the pr/det_frm branch July 2, 2024 18:37
@dennisklein dennisklein added this to the next milestone Jul 5, 2024
@coderabbitai coderabbitai bot mentioned this pull request Mar 19, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants