Allow for 3D box in FairBoxGenerator

[cvilelahep]

The current FairBoxGenerator allows only for generating particles over a plane as the depth of the box, dZ, is hard-coded to be 0.

In this pull request, the SetBoxXYZ method is overloaded to accept also two values of z (z1 and z2) from which dZ is computed in the same way as dX and dY.

---

Checklist:

• Followed the Contributing Guidelines

[coderabbitai]

Walkthrough

Walkthrough

The FairBoxGenerator class has been improved by updating the SetBoxXYZ method to include additional parameters, z1 and z2, allowing for precise definitions of box dimensions in the Z-axis. This enhancement increases flexibility while ensuring backward compatibility. Additionally, the contributors list now acknowledges Cristovao Vilela's contributions, complete with an ORCID identifier for better tracking.

Changes

File	Change Summary
fairroot/generators/FairBoxGenerator.cxx	Enhanced SetXYZ for better readability; modified SetBoxXYZ to include additional parameters z1 and z2, with updated internal logic for Z-dimension calculations.
fairroot/generators/FairBoxGenerator.h	Updated SetBoxXYZ method declaration to accept z1 and z2, improving flexibility while retaining the original method for backward compatibility.
CONTRIBUTORS	Added Cristovao Vilela to the list of contributors, including an ORCID identifier.
.zenodo.json	Added new entry for Cristovao Vilela with an ORCID identifier to the metadata structure.

---

Recent review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between 67ac496 and 08726b4.

Files ignored due to path filters (1)

• codemeta.json is excluded by !**/*.json

Files selected for processing (2)

• .zenodo.json (1 hunks)
• CONTRIBUTORS (1 hunks)

Files skipped from review due to trivial changes (1)

• CONTRIBUTORS

Additional comments not posted (1)

.zenodo.json (1)

165-169: Entry for Cristovao Vilela is correctly formatted and consistent.

The new entry for Cristovao Vilela follows the established structure and includes the necessary fields.

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

[coderabbitai]

Actionable comments posted: 0

Outside diff range, codebase verification and nitpick comments (1)

fairroot/generators/FairBoxGenerator.h (1)

115-116: Update method calls to match the new signature

The method calls to SetBoxXYZ in the following files do not match the new signature and need to be updated:

• examples/simulation/Tutorial4/macros/run_tutorial4.C
• examples/simulation/Tutorial4/macros/run_tutorial4_createMatrices.C
• examples/simulation/Tutorial4/macros/run_tutorial4_createGeometryFile.C

Ensure all calls to SetBoxXYZ match the new signature: SetBoxXYZ(Double32_t x1 = 0, Double32_t y1 = 0, Double32_t z1 = 0, Double32_t x2 = 0, Double32_t y2 = 0, Double32_t z2 = 0);

Analysis chain

LGTM! But verify the method usage in the codebase.

The method signature change is consistent with the implementation in the .cxx file.

However, ensure that all calls to SetBoxXYZ match the new signature.

Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify all method calls to `SetBoxXYZ` match the new signature.

# Test: Search for the method usage. Expect: Only occurrences of the new signature.
rg --type cpp -A 5 $'SetBoxXYZ'

Length of output: 3315

[karabowi]

@cvilelahep , can you please apply clang-format to the files?
Can you also update the license headers in both files by changing second line to:
* Copyright (C) 2014-2024 GSI Helmholtzzentrum fuer Schwerionenforschung GmbH *?

[coderabbitai]

Actionable comments posted: 0

Outside diff range, codebase verification and nitpick comments (1)

fairroot/generators/FairBoxGenerator.cxx (1)

109-128: Action Required: Update or Deprecate Overloaded Method SetBoxXYZ.

The overloaded SetBoxXYZ(x1, y1, x2, y2, z) method is still in use in the following example files and should be updated or deprecated to ensure consistency with the new method signature:

• examples/simulation/Tutorial4/macros/run_tutorial4_createMatrices.C
• examples/simulation/Tutorial4/macros/run_tutorial4.C
• examples/simulation/Tutorial4/macros/run_tutorial4_createGeometryFile.C

Additionally, the header file and implementation file still contain the old method signature:

• fairroot/generators/FairBoxGenerator.h
• fairroot/generators/FairBoxGenerator.cxx

Please update these instances to use the new SetBoxXYZ method that supports two z-coordinates.

Analysis chain

Approved: Enhancement to support 3D box in SetBoxXYZ.

The SetBoxXYZ method has been correctly updated to support two z-coordinates for generating particles within a 3D box.

Ensure that the usage of the overloaded method SetBoxXYZ(x1, y1, x2, y2, z) has been updated or deprecated as needed.

Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify the usage of the overloaded method `SetBoxXYZ(x1, y1, x2, y2, z)`.

# Test: Search for the function usage. Expect: No usage or deprecation comments.
rg --type cpp -A 5 $'SetBoxXYZ($_, $_, $_, $_, $_)'

Length of output: 52

---

Script:

#!/bin/bash
# Description: Verify the usage of the overloaded method `SetBoxXYZ`.

# Test: Search for the function usage. Expect: No usage or deprecation comments.
rg --type cpp 'SetBoxXYZ'

Length of output: 1238

[cvilelahep]

@cvilelahep , can you please apply clang-format to the files? Can you also update the license headers in both files by changing second line to: * Copyright (C) 2014-2024 GSI Helmholtzzentrum fuer Schwerionenforschung GmbH *?

@karabowi done.

[karabowi]

There is a problem with your function. It is in conflict with the old function SetXYZ():

/Users/alfaci/alfaci/workspace/FairRootGroup_FairRoot_PR-1576/examples/simulation/Tutorial4/macros/run_tutorial4.C:107:13: error: call to member function 'SetBoxXYZ' is ambiguous
    boxGen->SetBoxXYZ(-20., -20., 20., 20., 0.);

Maybe remove default values?

[cvilelahep]

There is a problem with your function. It is in conflict with the old function SetXYZ():

/Users/alfaci/alfaci/workspace/FairRootGroup_FairRoot_PR-1576/examples/simulation/Tutorial4/macros/run_tutorial4.C:107:13: error: call to member function 'SetBoxXYZ' is ambiguous
    boxGen->SetBoxXYZ(-20., -20., 20., 20., 0.);

Maybe remove default values?

Ah, of course, sorry for missing this. I removed the default values.

[karabowi]

@cvilelahep , sine your changes you should apply clang-format again.
Also, you should squash your commits into one and write a nice commit message following
our guidelines like G3 and G4.
Please also add your name to the CONTRIBUTORS file, in a separate commit.

[ChristianTackeGSI]

LGTM.

@karabowi we maybe should update one example with the new feature (and keep another with the old one)?

Most of the failing tests look good to me.

• One could be resolved by rebasing to latest dev branch.

[karabowi]

we maybe should update one example with the new feature (and keep another with the old one)?

@ChristianTackeGSI good idea, I have created an issue not to forget.