Skip to content

Remove f-string in logging statement - #255

Merged
skyw merged 4 commits into
mainfrom
skyw/remove-fstring-in-log
Jul 22, 2026
Merged

Remove f-string in logging statement#255
skyw merged 4 commits into
mainfrom
skyw/remove-fstring-in-log

Conversation

@skyw

@skyw skyw commented Jul 22, 2026

Copy link
Copy Markdown
Contributor

Changed them to lazy evaluation.

Through sequence of unfortunate events, we dropped ball and had device tensor in f-string in logging.

Having trivial host variable in f-string in logging is fine technically, but decided to enforce it.

skyw added 3 commits July 22, 2026 12:21
Signed-off-by: Hao Wu <skyw@nvidia.com>
Signed-off-by: Hao Wu <skyw@nvidia.com>
Signed-off-by: Hao Wu <skyw@nvidia.com>
@skyw
skyw requested a review from a team as a code owner July 22, 2026 19:33
@copy-pr-bot

copy-pr-bot Bot commented Jul 22, 2026

Copy link
Copy Markdown

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

@skyw
skyw requested review from mkhona-nvidia and removed request for mkhona-nvidia July 22, 2026 19:33
@skyw

skyw commented Jul 22, 2026

Copy link
Copy Markdown
Contributor Author

/ok to test 540af3f

@greptile-apps

greptile-apps Bot commented Jul 22, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR replaces f-string interpolation in all logging statements across 9 source files and 3 test files with %-style lazy evaluation, and adds the G004 ruff rule to enforce this pattern going forward.

  • All logging calls in optimizer implementations (adaptive_muon, muon, muon_utils, polargrad, scion, spel, soap, eig, registry) are updated consistently; format specifiers (e.g., %.6f) are correctly preserved where applicable.
  • pyproject.toml adds G004 to the ruff select list and introduces a logger-objects entry to teach ruff about absl.logging calls; the placement of logger-objects was flagged in a prior review thread.

Confidence Score: 5/5

Safe to merge — all changes are mechanical logging-style conversions with no logic changes, and the ruff G004 rule will prevent regressions.

Every change is a straightforward f-string-to-%-style substitution in logging calls. No logic, control flow, or data handling is modified. The ruff configuration addition is additive. No new device-to-host synchronizations are introduced.

No files require special attention beyond the previously discussed logger-objects TOML placement in pyproject.toml.

Important Files Changed

Filename Overview
pyproject.toml Added G004 rule to ruff select list and logger-objects setting; the logger-objects key placement was flagged in a previous review thread.
emerging_optimizers/soap/soap.py scale_factor debug log updated; scale_factor is a CUDA tensor but this was already discussed and accepted for debug-only logging.
emerging_optimizers/orthogonalized_optimizers/muon.py Two logging calls converted to lazy evaluation; sm_version (tuple of ints) and scalar/string optimizer params are all host-side values.
tests/test_spectral_clipping_utils.py Multiple logging calls updated; relative_polar_frobenius_diff is a 0-dim tensor used with %.6f inside a test function — acceptable per project rules.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A["logging.debug(msg, *args) called"] --> B{Is DEBUG level active?}
    B -- No --> C["No-op: format string never evaluated\n(args already evaluated by Python)"]
    B -- Yes --> D["Python calls msg % args\n(string formatting happens here)"]
    D --> E["Log record emitted to handler"]

    style C fill:#d4edda,stroke:#28a745
    style D fill:#fff3cd,stroke:#ffc107
    style E fill:#cce5ff,stroke:#004085
Loading

Reviews (2): Last reviewed commit: "Merge branch 'main' into skyw/remove-fst..." | Re-trigger Greptile

Comment thread emerging_optimizers/soap/soap.py
Comment thread pyproject.toml
@skyw

skyw commented Jul 22, 2026

Copy link
Copy Markdown
Contributor Author

/ok to test 14a502c

@skyw
skyw enabled auto-merge (squash) July 22, 2026 20:58
@skyw
skyw merged commit ff90af0 into main Jul 22, 2026
25 checks passed
@skyw
skyw deleted the skyw/remove-fstring-in-log branch July 22, 2026 21:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants