Make DoubleFloats & Quadmath weak dependencies (package extensions)#14
Merged
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (14)
✅ Files skipped from review due to trivial changes (2)
🚧 Files skipped from review as they are similar to previous changes (7)
📝 WalkthroughWalkthroughMove DoubleFloats/GenericFFT/Quadmath from direct deps into weakdeps + extension modules; add ext/ modules with precision, conversion, and FFT hooks; remove core re-exports and compat shims; update tests and benchmarks to import numeric types directly. ChangesWeak Dependency Extension Architecture
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/TransitionMatrices.jl`:
- Around line 79-81: The public export surface was reduced in
TransitionMatrices.jl (exports now only RotZYZ, Arb, Acb and no longer export
Double64 / Float128 / ComplexF128), so bump the package version in Project.toml
from "0.5.0" to "0.6.0" to reflect the breaking API change; open Project.toml
and update the version field to "0.6.0" (alternatively restore deprecated
aliases for Double64/Float128/ComplexF128 in src/TransitionMatrices.jl for one
cycle if you prefer not to bump).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 5bdeee27-151f-4928-b13d-a35a61b05042
📒 Files selected for processing (9)
Project.tomlext/TransitionMatricesDoubleFloatsExt.jlext/TransitionMatricesQuadmathExt.jlsrc/IITM/nfold.jlsrc/TransitionMatrices.jlsrc/compat/index.jlsrc/shapes/superspheroid.jltest/Project.tomltest/compat.jl
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #14 +/- ##
=======================================
Coverage 95.98% 95.98%
=======================================
Files 38 41 +3
Lines 5004 5004
=======================================
Hits 4803 4803
Misses 201 201
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
lucifer1004
force-pushed
the
weaken-precision-deps
branch
from
b865ebf to
aa7e3b5
Compare
…xtensions)
Float64-only users no longer load DoubleFloats, Quadmath (and thus the libquadmath C
library), or GenericFFT: all three are `[weakdeps]` activated via package extensions,
pulled in only when the user does `using DoubleFloats` / `using Quadmath` / `using
GenericFFT`. Verified: `using TransitionMatrices` alone loads none of the three
extensions, and a Float64 EBCM computation runs unchanged.
These are all opt-in extras — the core (Float64, FFTW, Arblib) path never needs them:
- ext/TransitionMatricesDoubleFloatsExt.jl: Double64 — precision(Complex{Double64}),
Arblib.set!(::Double64), and the Double64 `cbrt`/`∛` workaround.
- ext/TransitionMatricesQuadmathExt.jl: Float128 — Float128⇄Arb conversions,
precision(ComplexF128), the instance precision(::Float128) fix (Quadmath leaves it
unimplemented), and Arblib.set!(::Float128).
- ext/TransitionMatricesGenericFFTExt.jl: the generic-type azimuthal FFT for the n-fold
IITM — `_iitm_fft_capable(Complex{<:AbstractFloat})`, the GenericFFT plan, and the
generic `_apply_forward_dft!`. Without GenericFFT, Complex{Double64}/Complex{BigFloat}
n-fold IITM gracefully falls back to the direct azimuthal sum (correct, just no FFT
acceleration); ComplexF64 stays on FFTW and Acb on Arblib.dft! (both hard deps).
Note: weakening Quadmath alone would have no effect, because DoubleFloats lists Quadmath
in its own deps and so transitively loads it — they had to be weakened together for
Float64-only loads to drop libquadmath.
Arblib stays a hard dependency: the arbitrary-precision + special-function backbone
(gausslegendre for non-Float64, factorial(n>20) via gamma! in the Wigner-d functions —
reached even on the Float64 path — and Arb/Acb as first-class numeric types in the EBCM
Arb-matrix inversion and the IITM Acb FFT backend).
Re-exports of Double64 / Float128 / ComplexF128 are dropped (use the respective package);
Arb/Acb stay exported. Tests and the benchmark load the weak deps explicitly;
test/Project.toml and benchmark/Project.toml gain them. Full suite: 48,290 tests pass.
lucifer1004
force-pushed
the
weaken-precision-deps
branch
from
aa7e3b5 to
e0469b2
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Float64-only users no longer load DoubleFloats or Quadmath (and thus the libquadmath C library): both are now
[weakdeps]activated via package extensions, so they are pulled in only when the user doesusing DoubleFloats/using Quadmath.These two are purely user-opt-in numeric types (Double64 / Float128) — the core (Float64) path never needs them. Their type-specific glue moves to extensions:
cbrt(∛) workaround.Note: weakening Quadmath ALONE has no effect, because DoubleFloats (a hard dep) lists Quadmath in its own deps and so transitively loads it — both had to be weakened together for Float64-only loads to drop them. Verified: with only
using TransitionMatrices, neither extension is loaded and a Float64 EBCM computation runs unchanged.Arblib stays a hard dependency: it is the arbitrary-precision + special-function backbone (gausslegendre for non-Float64, factorial(n>20) via gamma! in the Wigner-d functions — reached even on the Float64 path — plus Arb/Acb as first-class numeric types in the EBCM Arb-matrix inversion and the IITM Acb FFT backend).
Re-exports of Double64 / Float128 / ComplexF128 are dropped (use the respective package); Arb/Acb stay exported. Tests updated to load the weak deps explicitly; test/Project.toml gains DoubleFloats and Quadmath. Full suite: 48,290 tests pass.
Summary by CodeRabbit
Chores
New Features
Bug Fixes