FairRootGroup / FairRoot

C++ simulation, reconstruction and analysis framework for particle physics experiments
http://fairroot.gsi.de
Other
57 stars 96 forks source link

[DRAFT/IDEA] Use std containers for FairModule::svList #1497

Closed ChristianTackeGSI closed 6 months ago

ChristianTackeGSI commented 6 months ago

FairModule::svList can very well be implemented using std::vector without all the casting.

And rename to fAllSensitiveVolumes.


Checklist:

coderabbitai[bot] commented 6 months ago
Walkthrough ## Walkthrough The recent updates focus on refining the handling of sensitive volumes within the FairRoot framework. By centralizing access to sensitive volumes through the `FairRunSim` instance, the changes eliminate redundant and outdated methods of volume management. This consolidation enhances the initialization and geometry setup processes, streamlining interactions with sensitive volumes across different components of the framework. Additionally, minor updates include copyright year revisions and the removal of comments related to Geant4 multi-threading workarounds. ## Changes | File(s) | Change Summary | |---------------------------------------------------------------------------------------------|-------------------------------------------------------------------------------------------------------------------------| | `fairroot/base/sim/FairDetector.cxx` | Altered iteration over sensitive volumes to utilize `FairRunSim::Instance()->fAllSensitiveVolumes` during initialization. | | `fairroot/base/sim/FairMCApplication.cxx` | Removed static thread local `svList` usage, directly accessing sensitive volumes from `fRun` in `InitGeometry`. | | `fairroot/base/sim/FairModule.cxx`, `fairroot/base/sim/FairModule.h` | Removed `svList` declaration, updated includes and handling of sensitive volumes, and removed redundant initializations. | | `fairroot/base/steer/FairRunSim.h` | Added sensitive volumes storage and updated friend declarations. | | `templates/.../NewDetector.cxx` (applies to all NewDetector template variations) | Updated copyright year and removed comments related to Geant4 MT mode workaround. |

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 - [X](https://twitter.com/intent/tweet?text=I%20just%20used%20%40coderabbitai%20for%20my%20code%20review%2C%20and%20it%27s%20fantastic%21%20It%27s%20free%20for%20OSS%20and%20offers%20a%20free%20trial%20for%20the%20proprietary%20code.%20Check%20it%20out%3A&url=https%3A//coderabbit.ai) - [Mastodon](https://mastodon.social/share?text=I%20just%20used%20%40coderabbitai%20for%20my%20code%20review%2C%20and%20it%27s%20fantastic%21%20It%27s%20free%20for%20OSS%20and%20offers%20a%20free%20trial%20for%20the%20proprietary%20code.%20Check%20it%20out%3A%20https%3A%2F%2Fcoderabbit.ai) - [Reddit](https://www.reddit.com/submit?title=Great%20tool%20for%20code%20review%20-%20CodeRabbit&text=I%20just%20used%20CodeRabbit%20for%20my%20code%20review%2C%20and%20it%27s%20fantastic%21%20It%27s%20free%20for%20OSS%20and%20offers%20a%20free%20trial%20for%20proprietary%20code.%20Check%20it%20out%3A%20https%3A//coderabbit.ai) - [LinkedIn](https://www.linkedin.com/sharing/share-offsite/?url=https%3A%2F%2Fcoderabbit.ai&mini=true&title=Great%20tool%20for%20code%20review%20-%20CodeRabbit&summary=I%20just%20used%20CodeRabbit%20for%20my%20code%20review%2C%20and%20it%27s%20fantastic%21%20It%27s%20free%20for%20OSS%20and%20offers%20a%20free%20trial%20for%20proprietary%20code)

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 .` - `Generate unit-tests 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 tests 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 tests.` - `@coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.` 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 a review. This is useful when automatic reviews are disabled for the repository. - `@coderabbitai resolve` resolve all the CodeRabbit review comments. - `@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. - The JSON schema for the configuration file is available [here](https://coderabbit.ai/integrations/coderabbit-overrides.v2.json). - 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/coderabbit-overrides.v2.json` ### CodeRabbit Discord Community Join our [Discord Community](https://discord.com/invite/GsXnASn26c) to get help, request features, and share feedback.
ChristianTackeGSI commented 6 months ago

@coderabbitai read the git commits in this PR and suggest improvements on the commit msg subject and body for each one. Make sure to state the reasons for your change suggestions.

use origin/dev as the base branch.

coderabbitai[bot] commented 6 months ago

[!TIP] For best results, initiate chat on the files or code changes.

@ChristianTackeGSI, for the commit 6a21820, I suggest the following improvements to make the commit message more informative and useful:

Suggested Commit Message

Subject: Refactor: Centralize sensitive volume management in FairRunSim

Body:
This commit relocates the management of sensitive volumes from the thread-local variable `svList` in the `FairModule` class to a centralized location within the `FairRunSim` class. This change aligns the handling of sensitive volumes with the concept of a "simulation session," ensuring that their management is directly associated with the lifecycle of a simulation run.

Rationale:
- Enhances the organization and accessibility of sensitive volumes by consolidating their management.
- Streamlines the process of accessing and processing sensitive volumes across different components of the simulation framework.
- Improves overall efficiency and maintainability of the codebase by reducing redundancy and potential for errors in volume management.

Impact:
This architectural change affects several components of the framework, including `FairDetector.cxx`, `FairMCApplication.cxx`, and `FairModule.cxx/h`. It necessitates updates in how sensitive volumes are accessed and managed, reflecting the centralized approach.

This modification was made in adherence to the contributing guidelines of the FairRoot project, considering the project's standards and practices.

Reasons for the Suggestion

This format follows best practices for commit messages, making them more valuable and informative for the project's history.

Analysis chain
Scripts executed The following scripts were executed for the analysis: --- Script: ```shell #!/bin/bash # Fetch the list of commits in the PR, comparing it against the origin/dev branch. git fetch origin dev git log --oneline origin/dev..HEAD ``` Length of output: 198
ChristianTackeGSI commented 6 months ago

Will re-open a new PR, because we want to test our AI review setup.