Add `setCreationMode` to `FairParSet`

[karabowi]

In rare cases, when parameteres deriving from FairParSet are not present in the input parameter file and are meant to be created during the run, FairParSet::init() throws an error message.

By using the newly introduced setter setCreationMode this message is suppressed.

This allows solution to issue #1541.

The PR also does some clean-up in FairParSet.

---

Checklist:

• [ ] Followed the Contributing Guidelines

Summary by CodeRabbit

• New Features

  • Introduced a new setCreateMode() method to manage creation modes.

• Deprecations

  • Deprecated unused methods fill() and store() in the FairParSet class.

• Improvements

  • Updated data types for better consistency and performance.
  • Enhanced logging for improved debugging and maintenance.

• Documentation

  • Provided guidance on deprecated APIs in the changelog.

[coderabbitai[bot]]

Walkthrough

Walkthrough

The 19.0.1 update for FairRoot focuses on modernizing the FairParSet class by deprecating unused methods, introducing a new setCreateMode() method, and updating data types from Int_t and Bool_t to int and bool for clarity and consistency. Additionally, logging improvements and code cleanups were made to enhance readability and maintainability.

Changes

File	Change Summary
CHANGELOG.md	Documented deprecation of unused methods, introduction of setCreateMode(), and guidance on deprecated APIs.
fairroot/parbase/FairParSet.cxx	Added creationMode variable, updated data types, improved logging, and replaced kFALSE/kTRUE with false/true.
fairroot/parbase/FairParSet.h	Updated data types, added creationMode variable, introduced new methods, and increased ClassDefOverride version.

Sequence Diagram(s) (Beta)

sequenceDiagram
    participant User
    participant FairParSet
    User ->> FairParSet: setCreateMode(true)
    FairParSet ->> FairParSet: creationMode = true
    User ->> FairParSet: isCreationMode()
    FairParSet -->> User: true

---

Recent review details

Configuration used: CodeRabbit UI Review profile: CHILL

Commits

Files that changed from the base of the PR and between 6f693eae1d6b68b2eef086bb03d99c1022b17256 and 51c73a4116a62f04bade26267cb3fb22d624e585.

Files selected for processing (3)

• CHANGELOG.md (2 hunks)
• fairroot/parbase/FairParSet.cxx (5 hunks)
• fairroot/parbase/FairParSet.h (4 hunks)

Additional context used

LanguageTool

CHANGELOG.md

[style] ~66-~66: ‘was in conflict with’ might be wordy. Consider a shorter alternative. (EN_WORDINESS_PREMIUM_WAS_IN_CONFLICT_WITH) Context: ...pped build switch BUILD_UNITTESTS, it was in conflict with the CMake standard switch [`BUILD_T...

---

[uncategorized] ~115-~115: A period might be missing here. (AI_EN_LECTOR_MISSING_PUNCTUATION_PERIOD) Context: ...` isn't searched for and not needed any more ### Example Changes in Experiment Repo...

---

[uncategorized] ~152-~152: You might be missing the article “the” here. (AI_EN_LECTOR_MISSING_DETERMINER_THE) Context: ... Stop run if false returned. * Remove sink from Tutorial3/MQ/sampler.cxx. * Fixed ...

---

[uncategorized] ~156-~156: You might be missing the article “an” here. (AI_EN_LECTOR_MISSING_DETERMINER_AN) Context: ...Notable Changes * Allow running without output sink. In this case even persistent bran...

---

[formatting] ~156-~156: Consider inserting a comma after an introductory phrase for better readability. (IN_THAT_CASE_COMMA) Context: ...es * Allow running without output sink. In this case even persistent branches would not be s...

---

[uncategorized] ~164-~164: A comma may be missing after the conjunctive/linking adverb ‘Also’. (SENT_START_CONJUNCTIVE_LINKING_ADVERB_COMMA) Context: ... the Base into the Online library. * Also the MBS/LMD related source classes were...

---

[uncategorized] ~166-~166: This verb does not appear to agree with the subject. Consider using a different form. (AI_EN_LECTOR_REPLACEMENT_VERB_AGREEMENT) Context: ...ne library. * If one of these classes are used in your code, you need to link...

---

[style] ~184-~184: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym. (ENGLISH_WORD_REPEAT_BEGINNING_RULE) Context: ... being set, rewrite it accordingly. * If your code has specific C++ needs, consi...

---

[misspelling] ~187-~187: This word is normally spelled as one. (EN_COMPOUNDS_ANTI_PATTERN) Context: ...USE_PATH_INFO CMake option. It was an anti-pattern. Don't use it! * Dropped `ROOT_GENERATE...

---

[uncategorized] ~196-~196: A punctuation mark might be missing here. (AI_EN_LECTOR_MISSING_PUNCTUATION) Context: ... You might have to add some #includess in your code. * Added a new build switch...

---

[uncategorized] ~200-~200: A comma might be missing here. (AI_EN_LECTOR_MISSING_PUNCTUATION_COMMA) Context: ...ll now require its external package dependencies FairMQ and Boost. * Boost serialization...

---

[grammar] ~203-~203: The word “opt-in” is a noun. The verb is spelled with a space. (NOUN_VERB_CONFUSION) Context: ...on Boost.Serialization. The user must opt-in to the previously baseline Boost serial...

---

[grammar] ~203-~203: You used an adverb (‘previously’) instead of an adjective, or a noun (‘baseline’) instead of another adjective. (A_RB_NN) Context: ...ialization. The user must opt-in to the previously baseline Boost serialization support by includ...

---

[typographical] ~216-~216: The conjunction “so that” does not have a comma in front. (SO_THAT_UNNECESSARY_COMMA) Context: ...y require some API, please file an issue, so that we can see how to handle this. * Depre...

---

[uncategorized] ~228-~228: Loose punctuation mark. (UNLIKELY_OPENING_PUNCTUATION) Context: ...tion. * FairRadMapManager::Instance, FairRadLenManager::Instance *Fair...

---

[uncategorized] ~232-~232: Loose punctuation mark. (UNLIKELY_OPENING_PUNCTUATION) Context: ...thing. * FairTask::*InputPersistance: There's no getter for `fInputPersis...

---

[uncategorized] ~239-~239: Loose punctuation mark. (UNLIKELY_OPENING_PUNCTUATION) Context: ...ConstructASCIIGeometry(T*, TString, U*), use FairModule::ConstructASCIIGeom...

---

[uncategorized] ~246-~246: This verb may not be in the correct tense. Consider changing the tense to fit the context better. (AI_EN_LECTOR_REPLACEMENT_VERB_TENSE) Context: ...rs should you need it. * Scheduled to remove without replacement in a future major r...

---

[uncategorized] ~265-~265: A comma might be missing here. (AI_EN_LECTOR_MISSING_PUNCTUATION_COMMA) Context: ...se` style naming. For those using the targets this change will not be visible. * Test...

---

[formatting] ~266-~266: If the ‘because’ clause is essential to the meaning, do not use a comma before the clause. (COMMA_BEFORE_BECAUSE) Context: ...ing Geant3 have been disabled by default, because those tests have a probability > 0 fo...

---

[style] ~268-~268: The word ‘anyways’ is informal American English. Did you mean “anyway”? (ANYWAYS) Context: ... for failing. If you want to run them anyways, pass -DENABLE_GEANT3_TESTING=ON to...

Markdownlint

CHANGELOG.md

123-123: Expected: 1; Actual: 0; Below (MD022, blanks-around-headings) Headings should be surrounded by blank lines

---

135-135: Expected: 1; Actual: 0; Below (MD022, blanks-around-headings) Headings should be surrounded by blank lines

---

143-143: Expected: 1; Actual: 0; Below (MD022, blanks-around-headings) Headings should be surrounded by blank lines

---

149-149: Expected: 1; Actual: 0; Below (MD022, blanks-around-headings) Headings should be surrounded by blank lines

---

155-155: Expected: 1; Actual: 0; Below (MD022, blanks-around-headings) Headings should be surrounded by blank lines

---

160-160: Expected: 1; Actual: 0; Below (MD022, blanks-around-headings) Headings should be surrounded by blank lines

---

248-248: Expected: 1; Actual: 0; Below (MD022, blanks-around-headings) Headings should be surrounded by blank lines

---

44-44: null (MD031, blanks-around-fences) Fenced code blocks should be surrounded by blank lines

---

47-47: null (MD031, blanks-around-fences) Fenced code blocks should be surrounded by blank lines

---

171-171: null (MD031, blanks-around-fences) Fenced code blocks should be surrounded by blank lines

---

173-173: null (MD031, blanks-around-fences) Fenced code blocks should be surrounded by blank lines

---

174-174: null (MD031, blanks-around-fences) Fenced code blocks should be surrounded by blank lines

---

176-176: null (MD031, blanks-around-fences) Fenced code blocks should be surrounded by blank lines

---

177-177: null (MD031, blanks-around-fences) Fenced code blocks should be surrounded by blank lines

---

181-181: null (MD031, blanks-around-fences) Fenced code blocks should be surrounded by blank lines

---

205-205: null (MD031, blanks-around-fences) Fenced code blocks should be surrounded by blank lines

---

208-208: null (MD031, blanks-around-fences) Fenced code blocks should be surrounded by blank lines

---

124-124: null (MD032, blanks-around-lists) Lists should be surrounded by blank lines

---

136-136: null (MD032, blanks-around-lists) Lists should be surrounded by blank lines

---

144-144: null (MD032, blanks-around-lists) Lists should be surrounded by blank lines

---

150-150: null (MD032, blanks-around-lists) Lists should be surrounded by blank lines

---

156-156: null (MD032, blanks-around-lists) Lists should be surrounded by blank lines

---

161-161: null (MD032, blanks-around-lists) Lists should be surrounded by blank lines

---

249-249: null (MD032, blanks-around-lists) Lists should be surrounded by blank lines

---

28-28: null (MD034, no-bare-urls) Bare URL used

---

30-30: null (MD034, no-bare-urls) Bare URL used

---

100-100: null (MD034, no-bare-urls) Bare URL used

---

119-119: null (MD034, no-bare-urls) Bare URL used

Additional comments not posted (10)

fairroot/parbase/FairParSet.h (5)

23-27: Update to use standard C++ types and addition of creationMode variable.

This change aligns with modern C++ standards and adds a new feature for managing creation mode, which is a good practice for enhancing functionality without breaking existing interfaces.

---

33-33: Constructor updated to use standard C++ boolean type.

Switching from Bool_t to bool is a positive change towards standardizing the codebase and reducing dependency on ROOT-specific types.

---

49-55: Updated method signatures to use standard C++ types.

The consistent use of standard types (int and bool) instead of ROOT types (Int_t, Bool_t) across these methods improves the code's portability and readability.

Also applies to: 64-65, 67-68, 70-71, 73-74

---

92-93: Deprecation of unused methods fill and store.

Marking unused methods as deprecated is a good practice. It warns other developers about the obsolescence of these methods, potentially guiding them towards newer or more efficient alternatives.

---

98-98: Class version updated in ClassDefOverride.

Incrementing the class version after making significant changes is necessary for maintaining compatibility with ROOT's I/O functionality.

fairroot/parbase/FairParSet.cxx (4)

34-39: Initialization of new member creationMode and default values for versions.

Proper initialization of class members in the constructor prevents undefined behavior and ensures that the object starts in a valid state.

---

Line range hint 54-77: Enhanced error handling in init method with respect to creationMode.

The conditional handling based on creationMode allows the system to operate in different configurations, which is useful for suppressing unnecessary error messages during initialization.

---

99-112: Use of logging instead of direct output to cout.

Replacing cout with logging statements is a best practice for production code, as it allows better control over the output and integrates with system-wide logging policies.

---

117-128: Resetting input versions and status in clear method.

Clearing or resetting the state of an object in the clear method ensures that the object can be reused without leftover data from previous operations, which is crucial for avoiding subtle bugs.

Proper copying of versions array in the copy constructor.

Ensuring that all relevant data members are correctly copied in the copy constructor is essential for the correct functioning of copied objects, especially when dealing with arrays or pointers.

CHANGELOG.md (1)

7-20: Updated changelog entries for version 19.0.1.

The changelog correctly summarizes the changes made in this version, including the introduction of new methods and the deprecation of unused ones. This is crucial for keeping the project's documentation up to date and providing clear communication to users and developers.

---

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
• Mastodon
• Reddit
• LinkedIn

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.

[ChristianTackeGSI]

Can you extract the chore / deprecate part into its own PR? I like those and would like to get them finished and merged first.

[ChristianTackeGSI]

After having thought about this whole thing a bit (see https://github.com/FairRootGroup/FairRoot/issues/1541#issuecomment-2158966426), I doubt that we actually should have yet another knob. (Yes, I proposed the knob thing, please excuse!) Not to mention, the "creation mode" as a bool sounds a bit strange. What is a truthy creation mode? What is a falsy creation mode?

And I really think by now, that the virtual init() is the customization point that users should use to handle all their cases about this part. We can try to make this even nicer / easier. All our cleanup PRs are going in that direction!

So alternative proposol (If you agrree): Repurpose this PR and drop the feat commit from it?

[karabowi]

@ChristianTackeGSI , I also do not like this knob name, I will work on this PR.