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.

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

avoid calling conductor #4247

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

avoid calling conductor #4247

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
48 changes: 21 additions & 27 deletions src/Rings/AbelianClosure.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -933,25 +933,22 @@

# Construct the map from `F` to an abelian closure `K` such that `gen(F)`
# is mapped to `x`.
# If `F` is a cyclotomic field with conductor `N` then assume that `gen(F)`
# is mapped to `QQAbFieldElem(gen(F), N)`.
# If `F` has conductor `N` then assume that `x.c == N` holds.
# If `F` is a cyclotomic field with conductor `N` then assume that
# `x == QQAbFieldElem(gen(F), N)`.
# (Use that the powers of this element form a basis of the field.)
function _embedding(F::QQField, K::QQAbField{AbsSimpleNumField},
x::QQAbFieldElem{AbsSimpleNumFieldElem})
C1, z = cyclotomic_field(1)
C1, _ = cyclotomic_field(1)

f = function(x::QQFieldElem)
return QQAbFieldElem(C1(x), 1)
end

finv = function(x::QQAbFieldElem; check::Bool = false)
if conductor(x) == 1
return Hecke.force_coerce_cyclo(C1, data(x))
elseif check
return
else
error("element has no preimage")
end
res = Hecke.force_coerce_cyclo(C1, data(x), Val(false))
!check && res === nothing && error("element has no preimage")
Copy link
Collaborator

Choose a reason for hiding this comment

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

Hm, if check == true and res === nothing, will this return nothing? Sorry, I am not good with logic.

Copy link
Member

Choose a reason for hiding this comment

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

This code is rather confusing to me as well. But so is the old code: it indeed also returns nothing when check is true and conductor(x) != 1 ?!

Copy link
Member

Choose a reason for hiding this comment

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

I guess what confuses me is that check apparently has a different meaning here than it has in e.g. our constructors. Since I don't know what the meaning of check is supposed to be here, I can't comment further.

Is it documented somewhere?

Copy link
Member Author

Choose a reason for hiding this comment

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

@fingolfin See my comment below: The function in question is internal and not documented (as well as the functions called by it). I think we could change the meaning of check to that in other situations.

return res

Check warning on line 951 in src/Rings/AbelianClosure.jl

View check run for this annotation

Codecov / codecov/patch

src/Rings/AbelianClosure.jl#L949-L951 

Added lines #L949 - L951 were not covered by tests
end

return MapFromFunc(F, K, f, finv)
Expand All @@ -967,17 +964,13 @@
end

finv = function(x::QQAbFieldElem; check::Bool = false)
if n % conductor(x) == 0
return Hecke.force_coerce_cyclo(F, data(x))
elseif check
return
else
error("element has no preimage")
end
res = Hecke.force_coerce_cyclo(F, data(x), Val(false))
!check && res === nothing && error("element has no preimage")
return res
end
else
# `F` is expected to be a proper subfield of a cyclotomic field.
n = conductor(x)
n = x.c
x = data(x)
Kn, = AbelianClosure.cyclotomic_field(K, n)
powers = [Hecke.coefficients(Hecke.force_coerce_cyclo(Kn, x^i))
Expand All @@ -990,24 +983,25 @@
end

finv = function(x::QQAbFieldElem; check::Bool = false)
n % conductor(x) == 0 || return false, zero(F)
# Write `x` w.r.t. the n-th cyclotomic field ...
g = gcd(x.c, n)
Kg, = AbelianClosure.cyclotomic_field(K, g)
x = Hecke.force_coerce_cyclo(Kg, data(x))
x = Hecke.force_coerce_cyclo(Kg, data(x), Val(false))
if x === nothing
!check && error("element has no preimage")
return

Check warning on line 992 in src/Rings/AbelianClosure.jl

View check run for this annotation

Codecov / codecov/patch

src/Rings/AbelianClosure.jl#L991-L992 

Added lines #L991 - L992 were not covered by tests
end
x = Hecke.force_coerce_cyclo(Kn, x)
# ... and then w.r.t. `F`
a = Hecke.coefficients(x)
fl, sol = can_solve_with_solution(c, matrix(QQ, length(a), 1, a); side = :right)
if fl
b = transpose(sol)
b = [b[i] for i in 1:length(b)]
return F(b)
elseif check
if !fl
!check && error("element has no preimage")
return
else
error("element has no preimage")
end
b = transpose(sol)
b = [b[i] for i in 1:length(b)]
return F(b)
end
end
return MapFromFunc(F, K, f, finv)
Expand Down
Loading