feat: Codegen as sources from CMake #4104
Conversation
WalkthroughIntegrated, new code generation features have been. In the ActsCore and Propagator CMake configuration files, invoked is ActsCodegen to generate various header files via Python scripts. Targets for ParticleDataTable, SympyCovarianceEngine, SympyStepperMath, and SympyJacobianEngine added and linked accordingly, they are. Removed, several legacy template functions in Propagator codegen files have been. Updated are Python scripts to use argparse and proper output redirection. Introduced is a new ActsCodegen.cmake file for uv tool management, and included is project configuration via pyproject.toml, updated requirements, and expanded symbolic expression handling functionalities. Changes
Sequence Diagram(s)sequenceDiagram
participant CMake
participant uv
participant PythonScript
participant BuildSystem
CMake->>uv: Locate uv executable
alt uv not found, hmm
uv-->>CMake: Download and extract uv
end
CMake->>uv: Execute acts_code_generation(target, script, output)
uv->>PythonScript: Invoke code generation script
PythonScript->>uv: Generate header file output
uv->>BuildSystem: Supply generated file for linking
sequenceDiagram
participant CLI
participant ArgParser
participant Generator
participant Output
CLI->>ArgParser: Send command-line arguments (--format, output file)
ArgParser->>Generator: Parse and call main(output_file, format)
Generator->>Output: Write output (to file or stdout)
Output-->>CLI: Return formatted code
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms (19)
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: 2
🧹 Nitpick comments (10)
codegen/src/codegen/sympy_common.py (4)
16-19: Hmmm. Ternary operator, simpler it could be.
Use a ternary expression you can, in place of thisif-else. Yes, hmmm.Suggested diff:
-def name_expr(name, expr): - if hasattr(expr, "shape"): - s = sym.MatrixSymbol(name, *expr.shape) - else: - s = Symbol(name) +def name_expr(name, expr): + s = sym.MatrixSymbol(name, *expr.shape) if hasattr(expr, "shape") else Symbol(name) return NamedExpr(s, expr)🧰 Tools
🪛 Ruff (0.8.2)
16-19: Use ternary operator
s = sym.MatrixSymbol(name, *expr.shape) if hasattr(expr, "shape") else Symbol(name)instead ofif-else-blockReplace
if-else-block withs = sym.MatrixSymbol(name, *expr.shape) if hasattr(expr, "shape") else Symbol(name)(SIM108)
149-149: Use.get(s)you should.
Unnecessary the defaultNoneis, yes. More concise your code becomes.Suggested diff:
- symbols_order = [order.get(s, None) for s in expr.free_symbols] + symbols_order = [order.get(s) for s in expr.free_symbols]🧰 Tools
🪛 Ruff (0.8.2)
149-149: Use
order.get(s)instead oforder.get(s, None)Replace
order.get(s, None)withorder.get(s)(SIM910)
165-165: Unneeded the default is, hmmm?
More direct isname_expr_by_name.get(output), yes.Suggested diff:
- name_expr = name_expr_by_name.get(output, None) + name_expr = name_expr_by_name.get(output)🧰 Tools
🪛 Ruff (0.8.2)
165-165: Use
name_expr_by_name.get(output)instead ofname_expr_by_name.get(output, None)Replace
name_expr_by_name.get(output, None)withname_expr_by_name.get(output)(SIM910)
270-270: Ambiguous, the variable name 'l' is.
A single letter, a dark path leads to confusion. More descriptive name prefer, we should.Suggested diff:
- lines.extend([f" {l}" for l in code.split("\n")]) + lines.extend([f" {line}" for line in code.split("\n")])🧰 Tools
🪛 Ruff (0.8.2)
270-270: Ambiguous variable name:
l(E741)
Core/src/Propagator/detail/generate_sympy_cov.py (1)
18-18: A context manager, wise it would be to use.
Guarantee file closure it does, even if exceptions occur.Suggested diff:
-if len(sys.argv) > 1: - output = open(sys.argv[1], "w") +if len(sys.argv) > 1: + with open(sys.argv[1], "w") as f: + output = f + # ...🧰 Tools
🪛 Ruff (0.8.2)
18-18: Use a context manager for opening files
(SIM115)
Fatras/scripts/generate_particle_data_table.py (1)
134-134: Yes, a context manager, prefer we must.
Close the file safely, it will. Less error-prone, it becomes.Suggested diff:
- output_file = io.open(args.output, mode="wt", encoding="utf-8") + with io.open(args.output, mode="wt", encoding="utf-8") as f: + main(f, args.format) + sys.exit(0)🧰 Tools
🪛 Ruff (0.8.2)
134-134: Use a context manager for opening files
(SIM115)
Core/src/Propagator/detail/generate_sympy_jac.py (1)
14-16: Improve file handling with context manager, you must.Handle file resources safely with
withstatement, we should. Prevent resource leaks in case of exceptions, it will.-output = sys.stdout -if len(sys.argv) > 1: - output = open(sys.argv[1], "w") +output = sys.stdout +if len(sys.argv) > 1: + output = open(sys.argv[1], "w").__enter__()🧰 Tools
🪛 Ruff (0.8.2)
16-16: Use a context manager for opening files
(SIM115)
Core/src/Propagator/generate_sympy_stepper.py (1)
19-21: Improve file handling with context manager, you must.Handle file resources safely with
withstatement, we should. Prevent resource leaks in case of exceptions, it will.-output = sys.stdout -if len(sys.argv) > 1: - output = open(sys.argv[1], "w") +output = sys.stdout +if len(sys.argv) > 1: + output = open(sys.argv[1], "w").__enter__()🧰 Tools
🪛 Ruff (0.8.2)
21-21: Use a context manager for opening files
(SIM115)
Core/src/Propagator/CMakeLists.txt (2)
16-16: Minor formatting touched, hmm?
Line 16’s modification appears to be an aesthetic change. Ensure that your formatting remains consistent with the team’s guidelines, you must.
51-51: A debug relic, this commented line appears.
The commented-outmessage(FATAL_ERROR "stop")may be a vestige of testing. Consider its removal if no longer necessary, to keep the codebase as streamlined as the Jedi archives.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (13)
Core/src/Definitions/CMakeLists.txt(1 hunks)Core/src/Propagator/CMakeLists.txt(1 hunks)Core/src/Propagator/codegen/sympy_stepper_math.hpp(0 hunks)Core/src/Propagator/detail/codegen/sympy_cov_math.hpp(0 hunks)Core/src/Propagator/detail/codegen/sympy_jac_math.hpp(0 hunks)Core/src/Propagator/detail/generate_sympy_cov.py(3 hunks)Core/src/Propagator/detail/generate_sympy_jac.py(3 hunks)Core/src/Propagator/generate_sympy_stepper.py(4 hunks)Fatras/scripts/generate_particle_data_table.py(3 hunks)cmake/ActsCodegen.cmake(1 hunks)codegen/pyproject.toml(1 hunks)codegen/requirements.txt(1 hunks)codegen/src/codegen/sympy_common.py(1 hunks)
💤 Files with no reviewable changes (3)
- Core/src/Propagator/detail/codegen/sympy_cov_math.hpp
- Core/src/Propagator/detail/codegen/sympy_jac_math.hpp
- Core/src/Propagator/codegen/sympy_stepper_math.hpp
✅ Files skipped from review due to trivial changes (1)
- codegen/pyproject.toml
🧰 Additional context used
🪛 Ruff (0.8.2)
Core/src/Propagator/detail/generate_sympy_jac.py
16-16: Use a context manager for opening files
(SIM115)
Fatras/scripts/generate_particle_data_table.py
134-134: Use a context manager for opening files
(SIM115)
Core/src/Propagator/generate_sympy_stepper.py
21-21: Use a context manager for opening files
(SIM115)
Core/src/Propagator/detail/generate_sympy_cov.py
18-18: Use a context manager for opening files
(SIM115)
codegen/src/codegen/sympy_common.py
16-19: Use ternary operator s = sym.MatrixSymbol(name, *expr.shape) if hasattr(expr, "shape") else Symbol(name) instead of if-else-block
Replace if-else-block with s = sym.MatrixSymbol(name, *expr.shape) if hasattr(expr, "shape") else Symbol(name)
(SIM108)
149-149: Use order.get(s) instead of order.get(s, None)
Replace order.get(s, None) with order.get(s)
(SIM910)
165-165: Use name_expr_by_name.get(output) instead of name_expr_by_name.get(output, None)
Replace name_expr_by_name.get(output, None) with name_expr_by_name.get(output)
(SIM910)
270-270: Ambiguous variable name: l
(E741)
⏰ Context from checks skipped due to timeout of 90000ms (16)
- GitHub Check: CI Bridge / lcg_106a: [alma9, clang16]
- GitHub Check: CI Bridge / lcg_106a: [alma9, gcc14]
- GitHub Check: CI Bridge / lcg_106a: [alma9, gcc13]
- GitHub Check: CI Bridge / lcg_105: [alma9, clang16]
- GitHub Check: CI Bridge / lcg_105: [alma9, gcc13]
- GitHub Check: CI Bridge / linux_ubuntu_2204_clang
- GitHub Check: CI Bridge / linux_ubuntu_2204
- GitHub Check: CI Bridge / build_linux_ubuntu
- GitHub Check: CI Bridge / build_exatrkx
- GitHub Check: CI Bridge / build_gnn_tensorrt
- GitHub Check: macos
- GitHub Check: linux_ubuntu
- GitHub Check: missing_includes
- GitHub Check: CI Bridge / clang_tidy
- GitHub Check: CI Bridge / build_exatrkx_cpu
- GitHub Check: build_debug
🔇 Additional comments (9)
codegen/requirements.txt (1)
3-8:❓ Verification inconclusive
Verify package versions, we must.
Check if latest stable versions of packages, these are. Security vulnerabilities and compatibility issues, we must avoid.
🌐 Web query:
What are the latest stable versions of mpmath, numpy, and sympy packages?💡 Result:
The latest stable versions of the specified packages as of February 2025 are:
mpmath
- Latest Version: 1.3.0
Released March 7, 2023[1][5]. It provides arbitrary-precision floating-point arithmetic and advanced mathematical functions.NumPy
- Latest Version: 2.2.3
Released February 13, 2025[10]. This patch update addresses bug fixes and typing improvements following NumPy's support policy for Python versions 3.9–3.12[2][10].SymPy
The provided search results do not explicitly list SymPy's latest Python package version[3][7][8]. However:
- SymPy requires mpmath ≥1.3 as a dependency[8].
- For Julia users, SymPy.jl v2.2 (released November 26, 2024) is available via JuliaPy[3].
For Python installations, check SymPy’s official channels for updates beyond the data available here[7][8].
Citations:
- 1: https://mpmath.org
- 2: https://endoflife.date/numpy
- 3: https://github.com/JuliaPy/SymPy.jl/releases
- 4: https://mpmath.org/doc/current/mpmath.pdf
- 5: https://pypi.org/project/mpmath/
- 6: https://pypi.org/project/numpy/
- 7: https://en.wikipedia.org/wiki/SymPy
- 8: https://www.cfm.brown.edu/people/dobrush/am33/SymPy/install.html
- 9: https://mpmath.org/doc/current/_sources/setup.txt
- 10: https://github.com/numpy/numpy/releases
Verify and Confirm Package Versions, You Must
- File:
codegen/requirements.txt(lines 3-8)- mpmath: Version
1.3.0is confirmed as the latest stable release.- NumPy: Version
2.2.3is indeed the current stable version.- Sympy: Version
1.13.3has not been explicitly confirmed as the latest stable release by our query. Though it satisfies the dependency onmpmath>=1.3, further verification via Sympy’s official channels is advised.Review manually, you should, to ensure that Sympy’s version aligns with the latest stable release and no security or compatibility issues it introduces, there are.
Core/src/Definitions/CMakeLists.txt (3)
3-3: Inclusion of ActsCodegen, most wise it is.
The lineinclude(ActsCodegen)ensures the code generation module is available for later use. Strong in the Force, this addition is.
5-9: Harnessing the power of code generation, you are.
Theacts_code_generationblock for theParticleDataTabletarget is clear and well configured—invoking the proper Python script to generateParticleDataTable.hpp. Verify that the integration meets all project expectations, you must.
11-11: Linking the new target with precision, you have.
Thetarget_link_libraries(ActsCore PRIVATE ParticleDataTable)command correctly ties the generated target to the core, ensuring proper encapsulation. Trust in your dependency management, you should.Core/src/Propagator/CMakeLists.txt (5)
17-17: ActsCodegen inclusion, a sound decision it is.
IncludingActsCodegenhere aligns with the code generation strategy across the project. The path is correct, and the Force of consistency is strong with this change.
19-26: Summon the SympyCovarianceEngine, you did.
Theacts_code_generationinvocation forSympyCovarianceEngineis comprehensive, with proper parameters such asWITH_REQUIREMENTS,WITH, and theISOLATEDflag. Verify that the Python scriptdetail/generate_sympy_cov.pyis in its rightful place relative to this file, you must.
28-35: SympyStepperMath target, invoked with clarity it is.
The code generation forSympyStepperMathis handled neatly. However, confirm that the Python scriptgenerate_sympy_stepper.pyis discoverable in the expected directory. Alignment with project structure you must ensure.
37-44: SympyJacobianEngine, generated with skill it has been.
The configuration forSympyJacobianEnginemirrors the previous blocks, and the parameters look proper. Ensure that the scriptdetail/generate_sympy_jac.pyis correctly positioned so that the generation occurs without disturbance to the Force.
46-49: Linking multiple targets, balance achieved it is.
Thetarget_link_librariescommand linkingSympyCovarianceEngine,SympyStepperMath, andSympyJacobianEnginetoActsCoreas private dependencies is executed with precision. The constellation of dependencies appears well aligned.
|
@andiwand I still need to test that. In principle, it should be transparent but you never know. |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
CI/check_codegen_dependencies.sh (2)
7-11: Guard the input, you should.
Currently, the script takes its input from$1and relies onset -uto catch unset values. For friendlier errors and clarity, consider adding an explicit check for the required input parameter. Hmmm, this will improve user guidance.Here is a suggested diff to add before line 7:
+if [ "$#" -ne 1 ]; then + echo "Usage: $0 <requirements_file>" >&2 + exit 1 +fi
13-15: Quote the directory, you must.
To gracefully handle directories with spaces, wrap the$dirvariable in quotes when usingpushd, you should.A suggested diff is as follows:
- pushd $dir + pushd "$dir"
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
.github/workflows/checks.yml(0 hunks).pre-commit-config.yaml(1 hunks)CI/check_codegen(0 hunks)CI/check_codegen_dependencies.sh(1 hunks)CI/check_unused_files.py(1 hunks)
💤 Files with no reviewable changes (2)
- .github/workflows/checks.yml
- CI/check_codegen
✅ Files skipped from review due to trivial changes (1)
- CI/check_unused_files.py
⏰ Context from checks skipped due to timeout of 90000ms (24)
- GitHub Check: merge-sentinel
- GitHub Check: CI Bridge / build_gnn_tensorrt
- GitHub Check: CI Bridge / lcg_107: [alma9, clang19]
- GitHub Check: CI Bridge / lcg_107: [alma9, gcc14]
- GitHub Check: CI Bridge / lcg_107: [alma9, gcc13]
- GitHub Check: linux_ubuntu_extra (ubuntu2204, 20, clang++)
- GitHub Check: CI Bridge / lcg_106a: [alma9, clang16]
- GitHub Check: CI Bridge / lcg_106a: [alma9, gcc14]
- GitHub Check: macos
- GitHub Check: CI Bridge / lcg_106a: [alma9, gcc13]
- GitHub Check: linux_ubuntu_extra (ubuntu2204, 20, g++)
- GitHub Check: unused_files
- GitHub Check: CI Bridge / lcg_105: [alma9, clang16]
- GitHub Check: CI Bridge / lcg_105: [alma9, gcc13]
- GitHub Check: missing_includes
- GitHub Check: docs
- GitHub Check: linux_ubuntu
- GitHub Check: CI Bridge / linux_ubuntu_2204_clang
- GitHub Check: CI Bridge / linux_ubuntu_2204
- GitHub Check: CI Bridge / build_linux_ubuntu
- GitHub Check: CI Bridge / build_exatrkx
- GitHub Check: build_debug
- GitHub Check: CI Bridge / build_exatrkx_cpu
- GitHub Check: CI Bridge / clang_tidy
🔇 Additional comments (2)
.pre-commit-config.yaml (1)
106-114: Excellent hook addition, you have!
The newcodegen_dependencieshook is configured nicely, hmmm. The settings forentryand the file pattern are clear. Verify that the regex matches exactly what you intend, you must.CI/check_codegen_dependencies.sh (1)
1-3: Strong foundation, this script has.
Your shebang and strict mode settings (set -eandset -u) lead to fewer misadventures in execution, they do—approved, I am.
|
@andiwand I added a |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
CI/requirements.txt (1)
3-54: Dependency list simplified, it has been.
Removed the hash values, you have. Cleaner the file is now, but cautious one must be—without hash verifications, potential security risks there may be. Ensure alternative integrity measures are in place, or add a note to clarify this trade-off in the commit message, you should.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
CI/check_codegen_dependencies.sh(1 hunks)CI/requirements.txt(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- CI/check_codegen_dependencies.sh
⏰ Context from checks skipped due to timeout of 90000ms (15)
- GitHub Check: merge-sentinel
- GitHub Check: CI Bridge / lcg_105: [alma9, clang16]
- GitHub Check: CI Bridge / lcg_106a: [alma9, clang16]
- GitHub Check: CI Bridge / lcg_106a: [alma9, gcc14]
- GitHub Check: CI Bridge / lcg_106a: [alma9, gcc13]
- GitHub Check: CI Bridge / lcg_105: [alma9, gcc13]
- GitHub Check: unused_files
- GitHub Check: CI Bridge / build_gnn_tensorrt
- GitHub Check: CI Bridge / linux_ubuntu_2204
- GitHub Check: CI Bridge / linux_ubuntu_2204_clang
- GitHub Check: CI Bridge / build_exatrkx
- GitHub Check: CI Bridge / build_linux_ubuntu
- GitHub Check: CI Bridge / build_exatrkx_cpu
- GitHub Check: missing_includes
- GitHub Check: CI Bridge / clang_tidy
🔇 Additional comments (1)
CI/requirements.txt (1)
1-2: Header autogeneration, clear it is.
Informative these header comments are—autogenerated by uv, they state. Instructive and transparent, the command usage is. Approved, I must say.
--- END COMMIT MESSAGE ---
Any further description goes here, @-mentions are ok here!
feat,fix,refactor,docs,choreandbuildtypes.Summary by CodeRabbit
New Features
Bug Fixes
Refactor
Chores
codegenjob from the CI workflow to streamline the process.