Skip to content

feat[next]: warn in case python is run without -O for non-embedded #2538

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
havogt wants to merge 4 commits into
base: main
Choose a base branch
from
Open

feat[next]: warn in case python is run without -O for non-embedded #2538

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
bde7b37
feat[next]: warn in case python is run without -O for non-embedded
havogt Mar 18, 2026
0ca516d
move to compile variant
havogt Mar 20, 2026
e98ddbf
add tests and fix stacklevels
havogt Mar 20, 2026
9798776
set stacklevel to point to internal function
havogt Mar 23, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions pyproject.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -284,6 +284,7 @@ markers = [
'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',
Copy link
Contributor Author

@havogt havogt Mar 20, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

unrelated fix that I sneak in...

'uses_applied_shifts: tests that require backend support for applied-shifts',
'uses_can_deref: tests that require backend support for can_deref builtin function',
'uses_composite_shifts: tests that use composite shifts in unstructured domain',
Expand Down
8 changes: 8 additions & 0 deletions src/gt4py/next/otf/compiled_program.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -569,6 +569,14 @@ def _compile_variant(
# key computed here
call_key: CompiledProgramsKey | None = None,
) -> None:
if __debug__:
Copy link
Contributor

@egparedes egparedes Mar 20, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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....

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"
Copy link

Copilot AI Mar 20, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Suggested change
" compiled backend. Consider running with `python -O` or setting the environment"
" non-embedded backend. Consider running with `python -O` or setting the environment"

Copilot uses AI. Check for mistakes.
" variable `PYTHONOPTIMIZE=1`.",
stacklevel=2,
Copy link

Copilot AI Mar 20, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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).

Suggested change
stacklevel=2,
stacklevel=3,

Copilot uses AI. Check for mistakes.
Copy link
Contributor

@egparedes egparedes Mar 20, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@havogt can you check if this suggestion make sense? I don't know by heart right now...

)

if not common.is_offset_provider(offset_provider):
if common.is_offset_provider_type(offset_provider):
raise ValueError(
Expand Down
Loading