Skip to content

fix dispatch to herk #1247

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 2 commits into from
Apr 1, 2025
Merged

fix dispatch to herk #1247

merged 2 commits into from
Apr 1, 2025

Conversation

@araujoms
Copy link
Collaborator

The herk wrapper was testing whether C was symmetric instead of Hermitian, thus preventing the dispatch from occurring for essentially all nonzero β.

@stevengj
Copy link
Member

LGTM. Do we have test coverage for this branch?

@ViralBShah
Copy link
Member

ViralBShah commented Mar 26, 2025 •
edited
Loading

Why are all the tests failing? I couldn't tell from a quick glance.

@KristofferC
Copy link
Member

My guess is that the lazy jll stuff broke them.

@araujoms
Copy link
Collaborator Author

I've found another typo, this time in syrk_wrapper!, I thought it would be better to add it here.

@ViralBShah
Copy link
Member

The dot-jl job seems to fail with:

🚨 Error: mkdir C:\cache: Cannot create a file when that file already exists.

@KristofferC
Copy link
Member

This happens to many windows buildkite workers in the ecosystem (and for Julia itself).

@dkarrasch
Copy link
Member

I'll merge this and move forward with cleaning up the herk_wrapper!.

@dkarrasch dkarrasch merged commit e64a3df into JuliaLang:master Apr 1, 2025
1 of 2 checks passed
@araujoms araujoms deleted the patch-1 branch April 1, 2025 13:02
@dkarrasch dkarrasch mentioned this pull request Apr 1, 2025
Merged
jishnub pushed a commit that referenced this pull request Apr 13, 2025
@dkarrasch
Clean up herk_wrapper! and add 5-arg tests (#1254)
e65e75c 
This method can only be reached with complex eltypes, and after
promotion of alpha and beta, they cannot be `Bool` anymore. Also, this
adds 5-arg `mul!` of which I'm not sure we had some (even though
coverage said it was covered, which is strange because it shouldn't due
to the issue fixed in #1247).
@araujoms araujoms mentioned this pull request Apr 22, 2025
Merged
@jishnub jishnub added backport 1.11 Change should be backported to the 1.11 release 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 22, 2025
jishnub pushed a commit that referenced this pull request May 12, 2025
@araujoms @jishnub
fix dispatch to herk (#1247)
d455b83 
(cherry picked from commit e64a3df)
@jishnub jishnub mentioned this pull request May 12, 2025
Merged
27 tasks
jishnub added a commit that referenced this pull request May 26, 2025
@jishnub @araujoms
@JeffBezanson @stevengj @WalterMadelim @KristofferC @dkarrasch @mcabbott
Backports release 1.12 (#1353)
7264a49 
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

Reviewers

@jishnub jishnub jishnub approved these changes

@dkarrasch dkarrasch dkarrasch approved these changes

Assignees

No one assigned

Labels

backport 1.10 Change should be backported to the 1.10 release backport 1.11 Change should be backported to the 1.11 release

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@araujoms @stevengj @ViralBShah @KristofferC @dkarrasch @jishnub