Skip to content

Add tqdm progress bar to Python solver#681

Merged
waltsims merged 4 commits intomasterfrom
add-tqdm-progress
Mar 27, 2026
Merged

Add tqdm progress bar to Python solver#681
waltsims merged 4 commits intomasterfrom
add-tqdm-progress

Conversation

@waltsims
Copy link
Copy Markdown
Owner

@waltsims waltsims commented Mar 27, 2026

Shows a progress bar during simulation when quiet=False (default). Suppressed with quiet=True. Uses tqdm with step count and rate.

Greptile Summary

This PR adds a tqdm progress bar to the Python solver's run() loop, giving users real-time feedback (step count, rate, and ETA) during simulations. The bar is shown by default (quiet=False) and suppressed with quiet=True, which was already an accepted parameter in kspaceFirstOrder and is now correctly threaded through to the Simulation class.

Key changes:

  • tqdm promoted to a module-level import in kspace_solver.py and declared as a runtime dependency (>=4.60).
  • Simulation.__init__ gains a quiet keyword argument (default False), stored as self.quiet.
  • run() wraps the step loop in a tqdm context manager when not self.quiet and remaining > 0; the total is set to remaining = self.Nt - self.t, so the bar is accurate even if some steps were already executed manually before run() was called.
  • Both the Python and C++ backends consistently honour the quiet flag (C++ path already forwarded it to cpp_sim.run()).
  • Two new smoke tests verify that the bar neither breaks a normal run nor suppresses results when quiet=True.

Confidence Score: 5/5

Safe to merge — the change is additive, correctly scoped to the Python backend's run loop, and covered by new smoke tests.

No P0 or P1 issues found. The tqdm integration is minimal and correct: total tracks remaining steps accurately, both backends honour quiet, the import is at module level, and the dependency version floor is sensible. All existing behaviour is preserved when quiet=True.

No files require special attention.

Important Files Changed

Filename Overview
kwave/solvers/kspace_solver.py Adds quiet parameter to Simulation.__init__ and wraps the run loop with a tqdm progress bar when quiet=False; tqdm is now a module-level import.
kwave/kspaceFirstOrder.py Threads the existing quiet parameter through to Simulation; the C++ backend already forwarded quiet to cpp_sim.run(), so both backends are consistent.
pyproject.toml Adds tqdm>=4.60 as a runtime dependency; the minimum version is reasonable given the stable context-manager API used.
tests/test_native_solver.py Adds TestProgressBar with two smoke tests covering quiet=False (bar shown) and quiet=True (bar suppressed); both assert on result shape.
uv.lock Lock file updated to include tqdm in the dependency graph; no concerns.

Sequence Diagram

sequenceDiagram
    participant User
    participant kspaceFirstOrder
    participant Simulation
    participant tqdm

    User->>kspaceFirstOrder: call(kgrid, medium, source, sensor, quiet=False)
    kspaceFirstOrder->>Simulation: __init__(..., quiet=False)
    kspaceFirstOrder->>Simulation: run()
    Simulation->>Simulation: setup() if not _is_setup
    Simulation->>Simulation: remaining = Nt - t
    alt quiet=False and remaining > 0
        Simulation->>tqdm: __enter__(total=remaining, desc="k-Wave", unit="step")
        loop t < Nt
            Simulation->>Simulation: step()
            Simulation->>tqdm: update(1)
        end
        Simulation->>tqdm: __exit__()
    else quiet=True
        loop t < Nt
            Simulation->>Simulation: step()
        end
    end
    Simulation-->>kspaceFirstOrder: result dict
    kspaceFirstOrder-->>User: result dict
Loading

Comments Outside Diff (1)

  1. uv.lock, line 763-773 (link)

    P1 Lock file not regenerated after adding tqdm dependency

    tqdm>=4.60 was added to pyproject.toml but the uv.lock was not properly regenerated. The k-wave-python package entry in the lock file is missing tqdm in two places:

    1. The dependencies section (lines 763–773) lists beartype, deepdiff, deprecated, h5py, jaxtyping, matplotlib, numpy, opencv-python, and scipy — but not tqdm.
    2. The [package.metadata] requires-dist section (lines 804–827) also does not include { name = "tqdm", specifier = ">=4.60" }.

    Because tqdm currently appears in the lock file only as a transitive dependency of gdown (an optional example extra) and ssfp (a transitive dep of the test extra), a plain pip install k-wave-python or uv sync without those extras would not install tqdm, causing an ImportError at runtime when quiet=False (the default).

    Additionally, running uv lock --check will fail because the lock file does not match pyproject.toml, which will break CI pipelines that verify lock-file integrity.

    Fix: Run uv lock to regenerate the lock file so tqdm is recorded as a direct runtime dependency of k-wave-python.

Reviews (2): Last reviewed commit: "Add tests for tqdm progress bar (quiet=F..." | Re-trigger Greptile

Shows a progress bar during simulation when quiet=False (default).
Suppressed with quiet=True. Uses tqdm with step count and rate.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@codecov
Copy link
Copy Markdown

codecov bot commented Mar 27, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 74.26%. Comparing base (785c0bd) to head (bcd62c0).
⚠️ Report is 5 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #681      +/-   ##
==========================================
+ Coverage   74.23%   74.26%   +0.02%     
==========================================
  Files          56       56              
  Lines        8012     8020       +8     
  Branches     1566     1568       +2     
==========================================
+ Hits         5948     5956       +8     
  Misses       1441     1441              
  Partials      623      623              
Flag Coverage Δ
3.10 74.26% <100.00%> (+0.02%) ⬆️
3.11 74.26% <100.00%> (+0.02%) ⬆️
3.12 74.26% <100.00%> (+0.02%) ⬆️
3.13 74.26% <100.00%> (+0.02%) ⬆️
macos-latest 74.23% <100.00%> (+0.02%) ⬆️
ubuntu-latest 74.23% <100.00%> (+0.02%) ⬆️
windows-latest 74.25% <100.00%> (+0.02%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

self.step()
remaining = self.Nt - self.t
if not self.quiet and remaining > 0:
from tqdm import tqdm
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P2 Lazy import for a required dependency

tqdm is now a required runtime dependency (added to pyproject.toml), so the deferred from tqdm import tqdm inside the conditional block is unnecessary. Moving it to the module-level imports at the top of the file keeps the import style consistent with the rest of the module and makes the dependency visible at a glance.

Or simply add from tqdm import tqdm at the top of kspace_solver.py alongside the other imports and remove the inline import here.

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

waltsims and others added 2 commits March 27, 2026 07:20
tqdm is a required dependency — no need for lazy import. Also regenerates
uv.lock so tqdm appears in the resolved dependency list for plain installs.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@waltsims
Copy link
Copy Markdown
Owner Author

@greptile-app

@waltsims waltsims merged commit 8d7bb8b into master Mar 27, 2026
93 checks passed
@waltsims waltsims deleted the add-tqdm-progress branch March 27, 2026 18:09
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.

1 participant