Skip to content

Modernize codebase - #117

Open
wheeheee wants to merge 16 commits into
masterfrom
wh/modernization
Open

wheeheee wants to merge 16 commits into
masterfrom
wh/modernization

Conversation

@wheeheee

@wheeheee wheeheee commented Jun 22, 2026 •

Copy link
Copy Markdown
Member
  • Updates syntax, uses more Base functions in 1.10, and some optimizations.
  • Removes inbounds for many functions, for safety. I'll add them back to some of the hot inner loops after a bit more benchmarking.
  • Played with leaving KA to decide on the workgroup (instead of 256) coz it seems faster on OpenCL, but will revert if benchmarks are posted showing otherwise
  • Fixed an ancient index out-of-bounds error in lift_inbounds!

@codecov

codecov Bot commented Jun 22, 2026 •

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 97.46377% with 21 lines in your changes missing coverage. Please review.
✅ Project coverage is 96.96%. Comparing base (a5ad928) to head (d17763d).

Files with missing lines Patch % Lines
src/Transforms/transforms_lifting.jl 91.54% 12 Missing ⚠️
src/Transforms/transforms_main.jl 73.33% 4 Missing ⚠️
src/Threshold/entropy.jl 91.30% 2 Missing ⚠️
src/Transforms/transforms_filter.jl 99.42% 1 Missing ⚠️
src/Util/util_main.jl 99.22% 1 Missing ⚠️
src/WT/wt_main.jl 96.96% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master     #117      +/-   ##
==========================================
+ Coverage   96.63%   96.96%   +0.33%     
==========================================
  Files          17       17              
  Lines        2052     1881     -171     
==========================================
- Hits         1983     1824     -159     
+ Misses         69       57      -12     

☔ View full report in Codecov by Harness.
📢 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.

@wheeheee
wheeheee force-pushed the wh/modernization branch 7 times, most recently from d49077f to 0555eba Compare June 29, 2026 10:57
@wheeheee

wheeheee commented Jun 29, 2026 •

Copy link
Copy Markdown
Member Author

Ok, the latest time benchmarks (under "bench summary") look pretty good, although the GitHub runner used for benchmarking is likely very noisy.
Mostly improvements (master / PR timings > 1), except for small regressions with (lifting) (i)wpt, doesn't really make sense why these specifically but it could just be a benchmarking artifact.

@JeffFessler JeffFessler mentioned this pull request Jun 29, 2026
@wheeheee
wheeheee force-pushed the wh/modernization branch 5 times, most recently from f633eed to 2135e25 Compare July 7, 2026 07:19
@wheeheee

wheeheee commented Jul 7, 2026 •

Copy link
Copy Markdown
Member Author

Benchmarks on this PR now almost universally beat master.

wheeheee added 13 commits July 8, 2026 00:29
(i)modwt_step!, improve modwt
fix gpu+cpu `modwt_step` tests
* use isnothing, new julia 1.10 base functions
* format benchmarks.jl
* format FILTERS and SCHEMES
- delete failed tests because of a bug in Base on 1.10, coz no point testing a base function in a package...

- readability, `count`, fix undefined variable error

- up/downsample bool

- improve denoise, nspin2circ, threshold

- bitshifts where appropriate, rounding_div4

- avoid mutating matchingpursuit input
testfunction

sinpi
shr for detailn
trailing_zeros for `maxtransformlevels`
ndigits
* steptype Union (predOrUp)
* indicate `filtermainloop...!`s are mutating
make lrange type stable
use more UnitRanges
move some inbounds
`@Const(bases)`, argument formatting
divrem
move if into loop and L >= 0
don't set workgroup size
gpu.jl testset for-loop
bitvector
reduce usage of `unsafe_vectorslice` in filter dwt code

modify `unsafe_dwt1level!`

reduce allocations from `dwt_transform_cols!`

reduce allocations from `dwt_transform_strided!`

checkbounds in stridedcopy!
wheeheee added 3 commits July 8, 2026 00:32
* annotate y with AbstractVector
* oopv::Union{Vector{T},Nothing}
* half in normalize
* split! and merge! checks
* stepj
* internal docs
* inline dwt1
mostly Integer -> Int
@wheeheee
wheeheee force-pushed the wh/modernization branch from 2135e25 to d17763d Compare July 7, 2026 16:33
@gummif

gummif commented Aug 5, 2026

Copy link
Copy Markdown
Member

Ok benchmark looking good. Is this ready for review? Quite difficult to review this mixed bag of random changes.

@wheeheee

wheeheee commented Aug 6, 2026

Copy link
Copy Markdown
Member Author

Admittedly touches many files in the same PR. Would you prefer if I split this up into 2 / 3 parts? I did make an effort to ensure that tests pass after each commit though, so it would be easy enough to cut off later commits.

The first 5 commits could be part 1, they're the more minor changes.

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.

2 participants