Skip to content

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

Merged
vtjnash merged 1 commit intomasterfrom
dk/cat
Aug 18, 2021
Merged

Avoid potential ambiguities for [h/v/hv]cat in packages#41511
vtjnash merged 1 commit intomasterfrom
dk/cat

Conversation

@dkarrasch
Copy link
Copy Markdown
Member

@dkarrasch dkarrasch commented Jul 8, 2021 •
edited
Loading

#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
@dkarrasch dkarrasch mentioned this pull request Jul 9, 2021
Closed
@pablosanjose pablosanjose mentioned this pull request Jul 12, 2021
Closed
@vtjnash vtjnash added the triage This should be discussed on a triage call label Jul 21, 2021
@dkarrasch dkarrasch mentioned this pull request Jul 22, 2021
Merged
@pablosanjose
Copy link
Copy Markdown
Contributor

pablosanjose commented Jul 22, 2021

Friendly bump [I need this! :-)]

@chriscoey
Copy link
Copy Markdown

chriscoey commented Jul 23, 2021

I do too 🙂

@andreasvarga
Copy link
Copy Markdown
Contributor

andreasvarga commented Jul 30, 2021

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
Copy Markdown
Member

KristofferC commented Jul 30, 2021

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
Copy Markdown
Member Author

dkarrasch commented Jul 30, 2021

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
Copy Markdown
Member

DilumAluthge commented Aug 3, 2021 •
edited
Loading

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 latency Latency label Aug 3, 2021
@DilumAluthge
Copy link
Copy Markdown
Member

DilumAluthge commented Aug 3, 2021

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
@sostock sostock mentioned this pull request Aug 18, 2021
Closed
@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
Copy Markdown
Member

JeffBezanson commented Sep 16, 2021

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

@dkarrasch dkarrasch mentioned this pull request Sep 21, 2021
Closed
LilithHafner pushed a commit to LilithHafner/julia that referenced this pull request Feb 22, 2022
@dkarrasch @LilithHafner
Avoid potential ambiguities for [h/v/hv]cat in packages (JuliaLang#…
74da0aa 
…41511)
LilithHafner pushed a commit to LilithHafner/julia that referenced this pull request Mar 8, 2022
@dkarrasch @LilithHafner
Avoid potential ambiguities for [h/v/hv]cat in packages (JuliaLang#…
d8aba28 
…41511)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

arrays [a, r, r, a, y, s] latency Latency linear algebra Linear algebra

Projects

None yet

Milestone

No milestone

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

@dkarrasch @pablosanjose @chriscoey @andreasvarga @KristofferC @DilumAluthge @JeffBezanson @vtjnash