Skip to content

Make Geant4 dataset downloads resumable and fail loudly - #97

Open
kavyawadhwa134 wants to merge 2 commits into
datasetsfrom
fix/dataset-download-resume
Open

Make Geant4 dataset downloads resumable and fail loudly#97
kavyawadhwa134 wants to merge 2 commits into
datasetsfrom
fix/dataset-download-resume

Conversation

@kavyawadhwa134

Copy link
Copy Markdown
Collaborator

Summary

Fixes four defects in files/datasets.py that made the Geant4 dataset install slow and unreliable.

The headline bug: a failed download was silently reported as success. concurrent.futures.wait() never inspects future results, so an exception inside _download_extract_dataset (a dropped connection, an HTTP error) was discarded. install_datasets() then returned normally and even printed Geant4 datasets size on disk after extraction: ..., despite a dataset being absent. The failure only resurfaced much later as an opaque Geant4 runtime error when the data couldn't be found.

I hit this for real while verifying the branch: G4NDL (1.06 GB, over half the entire bundle) failed mid-download, the install reported success with only 10 of 11 datasets present, and the full 1.06 GB had to be re-fetched from scratch on the next run — turning a ~45 min install into ~90 min.

Changes

  • Surface worker exceptions — iterate as_completed() and call .result() instead of bare wait(), so a failed dataset raises rather than masquerading as a complete install.
  • Retry with HTTP Range resume — a dropped connection now continues from the last received byte instead of discarding a partially downloaded multi-GB file. Falls back to restarting if the server ignores the range request.
  • 1 MiB chunks instead of 1 KiB — cuts ~1.1M loop iterations and pbar.update() calls per GB down to ~1k. At 1 KiB, G4NDL alone drove over a million tqdm updates.
  • Count len(chunk), not the nominal chunk size — the old code over-reported progress on every final short chunk.

Test plan

Verified against the live cern.ch/geant4-data server on macOS (Geant4 11.2.2):

  • Server honours Range requests — returns 206 with the exact byte count
  • Simulated a connection drop mid-download: first attempt died at ~106 KB, retry sent Range: bytes=106496- and resumed exactly there. Final size matched content-length exactly — no gap, no double-count.
  • Progress accounting is now exact: counted 290745 == server content-length 290745
  • Forced every download to 404 — install_datasets() now raises HTTPError instead of returning "success"
  • Full end-to-end run after the fix: all 11 datasets present, 100 events simulated, fields ['run', 'id', 'primaries', 'track'], ids 0→99

Not addressed here

Found while testing, left out to keep this focused — happy to open separate issues/PRs:

  • The README usage example calls app.run(n_events=100), but the method signature is run(self, primaries) — the documented example raises TypeError.
  • CMakeLists.txt sets no INSTALL_RPATH, so against a shared-library Geant4 the built extension has no LC_RPATH entries and import fails with Library not loaded: @rpath/libG4Tree.dylib unless DYLD_LIBRARY_PATH is set manually.
  • data_directory() appends the app name to application_directory(), which already ends in it (via platformdirs), yielding .../geant4_python_application/geant4_python_application/geant4/<version>/data.
  • dataset1.py, alt_datasets.py, old_datasets.py and 2datasets.py are unused drafts shipped inside the package.
  • MD5 verification present before this branch was dropped from the current datasets.py; downloads are no longer integrity-checked.

🤖 Generated with Claude Code

A failed dataset download was silently swallowed: concurrent.futures.wait()
never inspects future results, so install_datasets() reported success and
printed the on-disk size even when a dataset was missing. Observed live with
G4NDL (1.06 GB) — the install claimed success, the data was absent, and the
whole 1.06 GB had to be re-fetched from scratch on the next run.

- surface worker exceptions via as_completed()/result() instead of wait()
- retry with HTTP Range resume so a dropped connection continues from the
  last received byte rather than restarting a multi-GB transfer
- read in 1 MiB chunks instead of 1 KiB, cutting ~1.1M loop iterations and
  tqdm updates per GB down to ~1k
- count len(chunk) rather than the nominal chunk size, which over-reported
  progress on every final short chunk

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Sep 5, 2026

Copy link
Copy Markdown

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Team

Run ID: 95f2dd9f-f70d-4827-bdcf-c432fb6317b3

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

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