feat: introduce G4Trap converter #4072
Conversation
WalkthroughEnhanced, the support for converting trapezoid shapes has been. A new method for Changes
Sequence Diagram(s)sequenceDiagram
participant PVC as PhysicalVolumeConverter
participant SC as Geant4ShapeConverter
participant G4 as G4 Shape Object
PVC->>SC: Request trapezoid bounds conversion
alt Shape is G4Trap
SC-->>PVC: Returns bounds for G4Trap
else Shape is G4Trd
SC-->>PVC: Returns bounds for G4Trd
end
Suggested labels
Suggested reviewers
Poem
Tip 🌐 Web search-backed reviews and chat
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms (13)
🔇 Additional comments (4)
✨ Finishing Touches
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? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
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)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
Plugins/Geant4/src/Geant4Converters.cpp (1)
233-294: Strong with geometry, this implementation is!Handle all G4Trap parameters correctly, it does. Yet, optimize we can by pre-calculating common expressions.
Consider this optimization, you should:
- double hlX0 = (x1 + x2) * 0.5; - double hlX1 = 2 * z * tan(theta) * cos(phi) + (x3 + x4) * 0.5; - double hlY0 = y1; - double hlY1 = y2 + 2 * z * tan(theta) * sin(phi); + double tanTheta = tan(theta); + double zTanTheta = 2 * z * tanTheta; + double hlX0 = (x1 + x2) * 0.5; + double hlX1 = zTanTheta * cos(phi) + (x3 + x4) * 0.5; + double hlY0 = y1; + double hlY1 = y2 + zTanTheta * sin(phi);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
Plugins/Geant4/include/Acts/Plugins/Geant4/Geant4Converters.hpp(1 hunks)Plugins/Geant4/src/Geant4Converters.cpp(4 hunks)Tests/UnitTests/Plugins/Geant4/Geant4ConvertersTests.cpp(3 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: merge-sentinel
- GitHub Check: CI Bridge / build_gnn_tensorrt
🔇 Additional comments (6)
Plugins/Geant4/include/Acts/Plugins/Geant4/Geant4Converters.hpp (2)
128-128: Clear, the documentation now is!Specify the source shape type explicitly, you do. Good practice, this is.
136-142: Well-structured, the new method declaration is!Follow the established patterns, it does. Consistent with existing conversion methods, the signature and documentation are.
Tests/UnitTests/Plugins/Geant4/Geant4ConvertersTests.cpp (2)
146-146: Wise, the test renaming is!Distinguish between different trapezoid types clearly, it does now.
216-249: Thorough, the new test case is!Cover multiple configurations and verify essential properties, it does. Strong with the Force, this test is.
Plugins/Geant4/src/Geant4Converters.cpp (2)
376-376: Clear, the comment now is!Specify the source shape type explicitly, you do. Consistent with header changes, this is.
393-408: Well-structured, the G4Trap conversion block is!Follow established patterns and handle errors consistently, it does. Create surface correctly, it does.
|
This introduces the G4Trap converter and is fully based not he work of Lorenzo Damenti ([[email protected]]
--- END COMMIT MESSAGE ---
Any further description goes here, @-mentions are ok here!
feat,fix,refactor,docs,choreandbuildtypes.Summary by CodeRabbit
New Features
G4Trap, enhancing overall geometric processing.Tests
G4Trapconversion and updated existing test case forG4Trdto ensure reliable performance and validate the improved conversion functionality.