Skip to content

use smaller matrix size in peakflops on 32-bit #1255

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Apr 2, 2025
Merged

Conversation

JeffBezanson
Copy link
Member

This might help prevent failures like https://buildkite.com/julialang/julia-master/builds/46307#0195de97-a496-4daa-b71e-3124e3592505. Allocating several 128MB arrays on 32-bit certainly sounds like asking for an OOM.

@ViralBShah ViralBShah added backport 1.10 Change should be backported to the 1.10 release backport 1.12 Change should be backported to release-1.12 labels Apr 1, 2025
@ViralBShah
Copy link
Member

ViralBShah commented Apr 1, 2025

The actual tests seem to use 1024 so this shouldn't be causing a CI OOM. In any case, this is a good default for 32-bit systems.

https://github.com/JuliaLang/LinearAlgebra.jl/blob/master/test/generic.jl#L642

@ViralBShah
Copy link
Member

I see the actual issue comes from test/threads_exec.jl in the Julia tests that calls peakflops with no arguments. That test should probably also use 1024.

@ViralBShah ViralBShah merged commit e53b50c into master Apr 2, 2025
1 of 2 checks passed
@ViralBShah ViralBShah deleted the jb/peakflops32 branch April 2, 2025 05:31
@jishnub jishnub mentioned this pull request May 12, 2025
27 tasks
jishnub added a commit that referenced this pull request May 26, 2025
Backported PRs:
- [x] #1209 <!-- Remove `LinearAlgebra` qualifications in `cholesky.jl`
-->
- [x] #1230 <!-- Avoid materializing `diag` in `Diagonal` `kron` -->
- [x] #1240 <!-- Reduce `stable_muladdmul` branches in `generic
matvecmul!` -->
- [x] #1247 <!-- fix dispatch to herk -->
- [x] #1255 <!-- use smaller matrix size in `peakflops` on 32-bit -->
- [x] #1310 <!-- Only `@noinline` error path in `matmul_size_check` -->
- [x] #1267 <!-- Refine column ranges in `_isbanded_impl` -->
- [x] #1320 <!-- Copy matrices in `triu`/`tril` if no zero exists for
the `eltype` -->
- [x] #1324 <!-- Fix empty `Tridiagonal` broadcast -->
- [x] #1327 <!-- `iszero` check in hessenberg setindex -->
- [x] #1326 <!-- Fix multiplication with empty `HessenbergQ` -->
- [x] #1332 <!-- Unwrap triangular matrices in broadcast -->
- [x] #1337 <!-- Change `1:size` to `axes` in bidiag mul -->
- [x] #1342 <!-- `Char` uplo in `Bidiagonal` constructor -->
- [x] #1344 <!-- Update the docstring of ldiv! -->
- [x] #1335 <!-- Test: prune old LA based on ENV variable -->
- [x] #1346 <!-- Fix scaling unit triangular matrices -->
- [x] #1355 <!-- Add compat notice for `diagview` -->
- [x] #1349 <!-- Prune `LinearAlgebra` module in ambiguity test -->

Contains multiple commits, manual intervention needed:
- [x] #1238 <!-- Ensure positive-definite matrix in lapack posv test -->
- [x] #1298 <!-- Add `diagm` example -->
- [x] #1312 <!-- WIP: Try use method deletion instead of custom sysimage
-->
- [x] #1333 <!-- Make `fillstored!` public -->
- [x] #1331 <!-- Document SingularException throw for
inv(::AbstractMatrix) -->
- [x] #1350 <!-- Fix copy for partly initialized unit triangular -->

Non-merged PRs with backport label:
- [x] #1352 <!-- log for dense diagonal matrix with negative elements
-->
- [ ] #1305 <!-- Bounds-checking in triangular indexing branches -->

---------

Co-authored-by: Mateus Araújo <[email protected]>
Co-authored-by: Jeff Bezanson <[email protected]>
Co-authored-by: Steven G. Johnson <[email protected]>
Co-authored-by: WalterMadelim <[email protected]>
Co-authored-by: Kristoffer Carlsson <[email protected]>
Co-authored-by: Daniel Karrasch <[email protected]>
Co-authored-by: Michael Abbott <[email protected]>
@jishnub jishnub removed the backport 1.12 Change should be backported to release-1.12 label Jun 5, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport 1.10 Change should be backported to the 1.10 release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants