Skip to content

Make promote a no-op if inputs have same type #2405

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

Open
lgoettgens wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Make promote a no-op if inputs have same type #2405

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions src/Matrix.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1178,6 +1178,9 @@ end
function Base.promote(x::MatrixElem{S},
y::MatrixElem{T}) where {S <: NCRingElement,
T <: NCRingElement}
if S === T
return x, y
end
Comment on lines +1181 to +1183
Copy link
Copy Markdown
Member

@fingolfin fingolfin May 5, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

First off: if this fixes a concrete issue and both @lgoettgens and @thofma are happy with it, go ahead, I don't want to block progress.

That said, I have some questions (which could also be turned into an issue for later).

What if S === T but typeof(x) !== typeof(y)? I.e. different matrix types over the same ring type? We normally don't have that, unless something went wrong, so perhaps it is OK to ignore; it just seems odd.

More importantly, as @thofma already pointed out: what if S === T but base_ring(x) != base_ring(y) ?

Do we have a way to detect if the "base rings have a common promotion"?

One could of course do something stupid like this

Suggested change
if S === T
return x, y
end
if S === T
if base_ring(x) != base_ring(y)
R = parent(one(base_ring(x)) + one(base_ring(y))) # common base ring
base_ring(x) === R && return x, change_base_ring(R, y)
base_ring(y) === R && return change_base_ring(R, x), y
return change_base_ring(R, x), change_base_ring(R, y)
end
return x, y
end

Of course the way R is computed is a nasty hack. To do better I guess we would need a helper that takes two rings and returns their "promotion" -- or maybe this already exists?

I imagine that such a helper would always return one of its two argument, or else indicate a problem (by raising an error or returning nothing or so). It would have to be written carefully to match the actual promotion behaviour.

(BTW, it seems change_base_ring(R, x) always creates a new object, even if R is already the base ring of (the parent of) x. Is this intentional? Then it should be documented. If it is not, perhaps we should change that?)

Copy link
Copy Markdown
Member

@thofma thofma May 5, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is a problem, since the addition will throw in general (l. 1183), e.g. for finite fields.

I think we should leave possible improvements to a proper value based promotion system (I have an idea for it, but did not find the time yet to implement it). The changes are of a larger scale, since currently we do not keep track of enough information.

U = promote_rule_sym(S, T)
if U === S
return x, change_base_ring(base_ring(x), y)
Expand All @@ -1200,6 +1203,9 @@ end
function Base.promote(x::MatrixElem{S},
y::Vector{T}) where {S <: NCRingElement,
T <: NCRingElement}
if S === T
return x, y
end
U = promote_rule_sym(S, T)
if U === S
return x, map(base_ring(x), y)::Vector{S} # Julia needs help here
Expand All @@ -1222,6 +1228,9 @@ end
*(x::Vector, y::MatrixElem) = *(promote(x, y)...)

function Base.promote(x::MatElem{S}, y::T) where {S <: NCRingElement, T <: NCRingElement}
if S === T
return x, y
end
U = promote_rule_sym(S, T)
if U === S
return x, base_ring(x)(y)
Expand Down
3 changes: 3 additions & 0 deletions src/NCRings.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,9 @@ function promote_rule_sym(::Type{T}, ::Type{S}) where {T, S}
end

@inline function try_promote(x::S, y::T) where {S <: NCRingElem, T <: NCRingElem}
if S === T
return x, y
end
U = promote_rule_sym(S, T)
if S === U
return true, x, parent(x)(y)
Expand Down
Loading