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

Avoid potential ambiguities for [h/v/hv]cat in packages #41511

Merged
merged 1 commit into from
Aug 18, 2021
Merged

Conversation

dkarrasch
Copy link
Member

@dkarrasch dkarrasch commented Jul 8, 2021

#41394 has changed the method signature in concatenation functions involving UniformScalings. This triggers method ambiguities for matrix-related packages, including LinearMaps.jl (see JuliaLinearAlgebra/LinearMaps.jl#158). This PR resolves the ambiguity at the root.

Closes JuliaLinearAlgebra/LinearMaps.jl#158.

@dkarrasch dkarrasch added arrays [a, r, r, a, y, s] linear algebra Linear algebra labels Jul 8, 2021
@vtjnash vtjnash added the triage This should be discussed on a triage call label Jul 21, 2021
@pablosanjose
Copy link
Contributor

Friendly bump [I need this! :-)]

@chriscoey
Copy link

I do too 🙂

@andreasvarga
Copy link
Contributor

This would be very helpful for me too. See my last succesfull tests with DescriptorSystems with Julia 1.6 and the 174 failed tests with Julia 1.7 due to the [h/v/hv]cat issue.

@KristofferC KristofferC reopened this Jul 30, 2021
@KristofferC
Copy link
Member

Seems like this would be useful to people and is unlikely to have any real issues so I would say to just merge this.

@KristofferC KristofferC added the merge me PR is reviewed. Merge when all tests are passing label Jul 30, 2021
@dkarrasch
Copy link
Member Author

The issue is that the PR causing the ambiguity (#41394) has introduced heavy latency problems, so I think triage needs to discuss/decide whether we want to go forward merging this or go backward and revert #41394.

@DilumAluthge DilumAluthge removed the merge me PR is reviewed. Merge when all tests are passing label Aug 3, 2021
@DilumAluthge
Copy link
Member

DilumAluthge commented Aug 3, 2021

I'm removing the merge me label so that this can be discussed at the next triage call.

On a separate note, I restarted the two failing CI jobs, just so we can see if CI passes on this PR.

@DilumAluthge DilumAluthge added the compiler:latency Compiler latency label Aug 3, 2021
@DilumAluthge
Copy link
Member

Actually, I'll add the merge me label back.

If triage decides to revert #41394, but this PR has already been merged, presumably we can just revert this PR first and then revert #41394 (or revert them both in a single PR).

@DilumAluthge DilumAluthge added the merge me PR is reviewed. Merge when all tests are passing label Aug 3, 2021
@vtjnash vtjnash merged commit 2e82c0d into master Aug 18, 2021
@vtjnash vtjnash deleted the dk/cat branch August 18, 2021 21:06
@DilumAluthge DilumAluthge removed the merge me PR is reviewed. Merge when all tests are passing label Aug 25, 2021
@JeffBezanson JeffBezanson removed the triage This should be discussed on a triage call label Sep 16, 2021
@JeffBezanson
Copy link
Member

If this PR resolves the problem, I'm happy, but if anybody knows a remaining reason to revert #41394 speak up.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
arrays [a, r, r, a, y, s] compiler:latency Compiler latency linear algebra Linear algebra
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Clash between hvcat in LinearAlgebra/src/uniformscaling.jl and LinearMaps/src/blockmap.jl
8 participants