chore: Update particle data script #4081
Conversation
WalkthroughConverted, the particle data arrays have been. In the header and Python script, C-style arrays replaced with Changes
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
✨ 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 (
|
github-actions
bot
added
Component - Core
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
Fatras/scripts/generate_particle_data_table.py (1)
70-74: Strong with types, your code has become.Explicit std namespace usage, clarity it brings. Yet, one small improvement suggest, I do.
Remove extra spaces in Name type declaration, we should:
- ("Name", " const char* const", '"{}"'), + ("Name", "const char* const", '"{}"'),Core/src/Definitions/ParticleDataTable.hpp (2)
26037-26040: Consider double precision for mass values, you should.Though float serves us now, more precise calculations with double, we might need. For scientific computations, important this is.
-static const std::array<float, kParticlesCount> kParticlesMassMeV = { +static const std::array<double, kParticlesCount> kParticlesMassMeV = { // Og294~ - 0.0f, + 0.0,
39043-39046: Modernize further with std::string_view, we can.Though const char* const serves us well, more modern path with std::string_view exists. Better safety and interface, it provides.
+#include <string_view> -static const std::array<const char* const, kParticlesCount> kParticlesName = { +static const std::array<std::string_view, kParticlesCount> kParticlesName = {
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
Core/src/Definitions/ParticleDataTable.hpp(5 hunks)Fatras/scripts/generate_particle_data_table.py(4 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (13)
- GitHub Check: merge-sentinel
- GitHub Check: CI Bridge / build_exatrkx_cpu
- GitHub Check: CI Bridge / build_exatrkx
- GitHub Check: unused_files
- GitHub Check: missing_includes
- GitHub Check: macos
- GitHub Check: CI Bridge / build_gnn_tensorrt
- GitHub Check: linux_ubuntu_extra (ubuntu2204, 20, clang++)
- GitHub Check: CI Bridge / clang_tidy
- GitHub Check: linux_ubuntu_extra (ubuntu2204, 20, g++)
- GitHub Check: build_debug
- GitHub Check: docs
- GitHub Check: linux_ubuntu
🔇 Additional comments (5)
Fatras/scripts/generate_particle_data_table.py (2)
49-51: Wise choice, the inclusion of array header is.Strong with the Force, this change is. Modern C++ practices, it follows.
82-83: Powerful, the std::array transformation is.From the dark side of C-style arrays to the light of modern C++, moved we have. Safer and more powerful, this approach is.
Core/src/Definitions/ParticleDataTable.hpp (3)
15-15: Wise addition of std::array header, this is!Necessary for the modern C++ features we seek, this include is. Well-placed in the include order, it stands.
25-28: Strong with modern C++, this change is!From the dark side of C-style arrays to the light side of std::array, we have moved. Better type safety and compile-time bounds checking, this brings.
13031-13034: Balanced and harmonious, this transformation is!The Force flows through std::int16_t, it does. Wise choice for particle charges, this type is.
|
@AJPfleger you ok with this? |
|
particlepackage version used to run (also you can run this withuv runnow)std::type_tas per our guidelinesstd::arrayover C-style arrays--- END COMMIT MESSAGE ---
Any further description goes here, @-mentions are ok here!
feat,fix,refactor,docs,choreandbuildtypes.Summary by CodeRabbit
Refactor
Chores