feat[next]: warn in case python is run without -O for non-embedded#2538
feat[next]: warn in case python is run without -O for non-embedded#2538havogt wants to merge 3 commits intoGridTools:mainfrom
Conversation
There was a problem hiding this comment.
Pull request overview
Adds a runtime warning in the gt4py.next frontend to alert users when executing with a compiled backend while Python is running in non-optimized mode (no -O / PYTHONOPTIMIZE), which can significantly affect performance due to extra debug-time checks.
Changes:
- Introduces a shared
_PYTHON_OPTIMIZE_WARNINGmessage constant. - Emits the warning when calling a compiled
Programin__debug__mode. - Emits the warning when directly calling a non-embedded compiled
FieldOperatorin__debug__mode.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
| 'requires_atlas: tests that require `atlas4py` bindings package', | ||
| 'requires_dace: tests that require `dace` package', | ||
| 'requires_gpu: tests that require a NVidia GPU (`cupy` and `cudatoolkit` are required)', | ||
| 'requires_jax: tests that require `jax` package', |
There was a problem hiding this comment.
unrelated fix that I sneak in...
There was a problem hiding this comment.
Pull request overview
Adds a runtime warning to alert users when running GT4Py-NEXT with assertions/debug checks enabled (i.e., without -O) in non-embedded execution paths, and aligns pytest marker configuration with existing test usage.
Changes:
- Emit a
warnings.warn(...)during variant compilation when__debug__is enabled. - Register the
requires_jaxpytest marker inpyproject.toml.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
src/gt4py/next/otf/compiled_program.py |
Warns at compile-time when Python isn’t running with optimizations enabled. |
pyproject.toml |
Declares the requires_jax pytest marker used by tests. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if __debug__: | ||
| warnings.warn( | ||
| "Python is not running in optimized mode, which may impact performance when using a" | ||
| " compiled backend. Consider running with `python -O` or setting the environment" |
There was a problem hiding this comment.
The warning text says it may impact performance when using a “compiled backend”, but this code path runs for any non-embedded backend, including pure-Python backends like roundtrip (see src/gt4py/next/program_processors/runners/roundtrip.py). Consider rewording to “non-embedded backend” (or only emitting for truly compiled backends) to avoid misleading users.
| " compiled backend. Consider running with `python -O` or setting the environment" | |
| " non-embedded backend. Consider running with `python -O` or setting the environment" |
| "Python is not running in optimized mode, which may impact performance when using a" | ||
| " compiled backend. Consider running with `python -O` or setting the environment" | ||
| " variable `PYTHONOPTIMIZE=1`.", | ||
| stacklevel=2, |
There was a problem hiding this comment.
With stacklevel=2 this warning will typically point at an internal call site in compiled_program.py (e.g., the _compile_variant caller) rather than the public API entry points (Program.__call__ / Program.compile) that users interact with. Consider increasing the stacklevel so the warning location is more actionable (and remains stable across call sites).
| stacklevel=2, | |
| stacklevel=3, |
There was a problem hiding this comment.
@havogt can you check if this suggestion make sense? I don't know by heart right now...
egparedes
left a comment
egparedes
left a comment
There was a problem hiding this comment.
LGTM. I have a couple of non-blocking comments so I'm just approving the PR now.
| # key computed here | ||
| call_key: CompiledProgramsKey | None = None, | ||
| ) -> None: | ||
| if __debug__: |
There was a problem hiding this comment.
Optional suggestion: since this code has nothing to do with the compilation itself, we could move it to the compile_variant_hook default implementation, since it kind of fits better for unrelated aspects like the metrics or this warning and will have the same effect....
| "Python is not running in optimized mode, which may impact performance when using a" | ||
| " compiled backend. Consider running with `python -O` or setting the environment" | ||
| " variable `PYTHONOPTIMIZE=1`.", | ||
| stacklevel=2, |
There was a problem hiding this comment.
@havogt can you check if this suggestion make sense? I don't know by heart right now...
No description provided.