Skip to content
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

Revert "Avoid materializing arrays in bidiag matmul" #55737

Merged
merged 1 commit into from
Sep 13, 2024

Conversation

giordano
Copy link
Contributor

Reverts #55450. @jishnub suggested reverting this PR to fix #55727.

@giordano giordano added the kind:revert This reverts a previously merged PR. label Sep 11, 2024
@jishnub
Copy link
Contributor

jishnub commented Sep 11, 2024

I'm not sure if the PR introduces this issue, but that's where I saw this first. Given that this happens unpredictably, I wonder if tests passing here will necessarily suggest that the issue is fixed. After all, tests passed on the PR as well.

@giordano giordano marked this pull request as draft September 11, 2024 20:09
@giordano
Copy link
Contributor Author

For the record, I'm stress-testing this PR by restarting CI several times (not great, but with an intermittent issue and no one finding the underlying culprit I don't see many other options): in 10 rereruns I haven't seen the error reported in #55727 yet

@giordano giordano marked this pull request as ready for review September 13, 2024 22:07
@giordano giordano merged commit 2ee6551 into master Sep 13, 2024
8 checks passed
@giordano giordano deleted the revert-55450-jishnub/bidiag_naivemul branch September 13, 2024 22:47
@giordano
Copy link
Contributor Author

With this PR I couldn't reproduce any error

Base.runtests("LinearAlgebra/addmul"; seed=0x8fd036c919ca6b04d09997a08163bff1)
Base.runtests("LinearAlgebra/addmul"; seed=0x86c87ef1d9841135b01925d2cddb12f3)

nor getting any failure by launching

Base.runtests("LinearAlgebra/addmul")

a few hundreds times on a server. I'd say both of these are strong indications that #55450 was indeed the culprit (or exposed an underlying issue?).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind:revert This reverts a previously merged PR.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

LinearAlgebra/addmul all-NAN batman?
2 participants