Skip to content

Commit

Permalink
Fix some issues found by Markus Kirschmer
Browse files Browse the repository at this point in the history
  • Loading branch information
thofma committed Dec 5, 2023
1 parent 776e633 commit 37cd5a7
Show file tree
Hide file tree
Showing 3 changed files with 20 additions and 15 deletions.
18 changes: 10 additions & 8 deletions src/NumFieldOrd/NfOrd/FracIdeal.jl
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ end
################################################################################

function iszero(x::NfAbsOrdFracIdl)
return iszero(numerator(x))
return iszero(numerator(x, copy = false))
end

#################################################################################
Expand Down Expand Up @@ -234,12 +234,14 @@ order(a::NfAbsOrdFracIdl) = a.order
Returns the inverse of the basis matrix of $I$.
"""
function basis_mat_inv(a::NfAbsOrdFracIdl)
if isdefined(a, :basis_mat_inv)
return deepcopy(a.basis_mat_inv)
else
function basis_mat_inv(a::NfAbsOrdFracIdl; copy::Bool = true)
if !isdefined(a, :basis_mat_inv)
a.basis_mat_inv = inv(basis_matrix(a))
end
if copy
return deepcopy(a.basis_mat_inv)
else
return a.basis_mat_inv

Check warning on line 244 in src/NumFieldOrd/NfOrd/FracIdeal.jl

View check run for this annotation

Codecov / codecov/patch

src/NumFieldOrd/NfOrd/FracIdeal.jl#L244

Added line #L244 was not covered by tests
end
end

Expand Down Expand Up @@ -627,7 +629,7 @@ function +(A::NfAbsOrdIdl{S, T}, B::NfAbsOrdFracIdl{S, T}) where {S <: NumField,
return fractional_ideal(order(A), A)
end
n1 = A*denominator(B)
n = n1 + numerator(B)
n = n1 + numerator(B, copy = false)
return n//denominator(B)
end

Expand All @@ -645,7 +647,7 @@ function +(A::NfAbsOrdFracIdl{S, T}, B::Hecke.NfAbsOrdFracIdl{S, T}) where {S <:
d = lcm(denominator(A), denominator(B))
ma = div(d, denominator(A))
mb = div(d, denominator(B))
return (numerator(A)*ma + numerator(B)*mb)//d
return (numerator(A, copy = false)*ma + numerator(B, copy = false)*mb)//d
end

function *(x::T, y::NfAbsOrd{S, T}) where {S <: NumField, T <: NumFieldElem}
Expand Down Expand Up @@ -792,7 +794,7 @@ function in(x::nf_elem, y::NfOrdFracIdl)
M = zero_matrix(FlintZZ, 1, degree(O))
t = FakeFmpqMat(M)
elem_to_mat_row!(t.num, 1, t.den, x)
v = t*basis_mat_inv(O)
v = t*basis_mat_inv(O, copy = false)
v = v*B

return v.den == 1
Expand Down
11 changes: 4 additions & 7 deletions src/NumFieldOrd/NfOrd/Ideal/Ideal.jl
Original file line number Diff line number Diff line change
Expand Up @@ -268,23 +268,20 @@ checked whether $M$ defines an ideal (expensive). If `M_in_hnf` is set, then it
that $M$ is already in lower left HNF.
"""
function ideal(O::NfAbsOrd, M::ZZMatrix; check::Bool = false, M_in_hnf::Bool = false)
!M_in_hnf ? x = _hnf(M, :lowerleft) : nothing #sub-optimal, but == relies on the basis being thus
#_trace_call(;print = true)
I = NfAbsOrdIdl(O, M)
x = !M_in_hnf ? _hnf(M, :lowerleft) : M #sub-optimal, but == relies on the basis being thus
I = NfAbsOrdIdl(O, x)
# The compiler stopped liking this recursion??
# if check
# J = ideal(O, basis(I))
# @assert J == I
# end

return I
end

function _ideal(O::NfAbsOrd, M::ZZMatrix, M_in_hnf::Bool = false)
!M_in_hnf ? x = _hnf(M, :lowerleft) : nothing #sub-optimal, but == relies on the basis being thus
x = !M_in_hnf ? _hnf(M, :lowerleft) : M #sub-optimal, but == relies on the basis being thus
#_trace_call(;print = true)
I = NfAbsOrdIdl(O, M)

I = NfAbsOrdIdl(O, x)
return I
end

Expand Down
6 changes: 6 additions & 0 deletions test/NfOrd/Ideal.jl
Original file line number Diff line number Diff line change
Expand Up @@ -276,5 +276,11 @@
Hecke.assure_2_normal(I)
@test isdefined(I, :gen_two)

# Some issue with ideal(O, M)
K, a = quadratic_field(-1)
O = maximal_order(K)
@test basis_matrix(ideal(O, representation_matrix(O(a)))) == identity_matrix(ZZ, 2)

include("Ideal/Prime.jl")

end

0 comments on commit 37cd5a7

Please sign in to comment.