Precise Johnson solids using serialization

[alexej-jordan]

This PR adds precise Johnson solids over fields not supported by polymake's QuadraticExtension. As the construction uses the V-description, pre-computed matrices are loaded. This required additional serialization methods for EmbeddedField, EmbeddedFieldElem and underlying embedding types.

[codecov]

Codecov Report

Merging #3035 (c7be419) into master (ad349a2) will increase coverage by 0.39%.
Report is 26 commits behind head on master.
The diff coverage is 84.00%.

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #3035      +/-   ##
==========================================
+ Coverage   80.09%   80.49%   +0.39%     
==========================================
  Files         491      523      +32     
  Lines       69349    70413    +1064     
==========================================
+ Hits        55547    56679    +1132     
+ Misses      13802    13734      -68     

Files	Coverage Δ
...edralGeometry/Polyhedron/standard_constructions.jl	96.24% <100.00%> (+0.59%)	⬆️
src/Serialization/Fields.jl	91.63% <82.22%> (-2.44%)	⬇️

... and 83 files with indirect coverage changes

[thofma]

Regarding rewriting

function save_object(s::SerializerState, E::FieldEmbeddingTypes)

I am bit worried about using _absolute_index. This relies on some ordering of zeros of roots of polynomials. This is consistent within one session, but we do not guarantee some persistent behaviour across sessions. (It will probably work now, but we don't guarantee that it will work in the future.)

Was the idea to have only one implementation for all four different types?

[antonydellavecchia]

looks good, one small comment

[alexej-jordan]

Regarding rewriting

function save_object(s::SerializerState, E::FieldEmbeddingTypes)

I am bit worried about using _absolute_index. This relies on some ordering of zeros of roots of polynomials. This is consistent within one session, but we do not guarantee some persistent behaviour across sessions. (It will probably work now, but we don't guarantee that it will work in the future.)

That was what we were worried about, too, hence we need your insight.

Was the idea to have only one implementation for all four different types?

The idea was to at least have serialization working for all four types. The newly added types usually could not be constructed with a complex_embedding method because none is defined for the corresponding number field types.

[thofma]

I will think about it.

[thofma]

It appears I need to write a few lines of code to make this a bit easier. I will come back to you once this is done.

[thofma]

I pushed the improved serialization with extensive tests. This needs thofma/Hecke.jl#1296, which should be available in a few hours. I will restart CI once it is.

[thofma]

I guess we need to recompute those *.mat files to work with the modified serialization?

[alexej-jordan]

Thank you for helping out on this! I managed to successfully save and load the matrices. Unfortunately, there is an exception:

julia> Qx, x = QQ["x"]
(Univariate polynomial ring in x over QQ, x)

julia> NF, qr8 = number_field(x^4 - 8, "a")
(Number field of degree 4 over QQ, a)

julia> ENF, qre8 = Hecke.embedded_field(NF, real_embeddings(NF)[2])
(Embedded field
Number field of degree 4 over QQ
at
Complex embedding corresponding to 1.68 of number field, a)

julia> emb = embedding(ENF)
Complex embedding corresponding to 1.68
  of number field with defining polynomial x^4 - 8
    over rational field

julia> save("emb.emb", emb)

julia> Oscar.reset_global_serializer_state()
Dict{Base.UUID, Any}()

julia> load("emb.emb")
┌ Warning: Attempting to load file stored using a DEV version with commit 7dbce862a671f43911ccffb1c4c5ada5738db1a0
└ @ Oscar ~/.julia/dev/Oscar/src/Serialization/main.jl:657
ERROR: Given approximation not close enough to a root. Possible roots are:
   -1.68 + i * 0.00, 1.68 + i * 0.00, 0.00 + i * 1.68, 0.00 + i * -1.68

Stacktrace:
  [1] error(s::String)
    @ Base ./error.jl:35
  [2] _find_nearest_complex_embedding(K::AnticNumberField, x::acb)
    @ Hecke ~/.julia/packages/Hecke/78SWd/src/NumField/ComplexEmbeddings/NfAbs.jl:272
  [3] complex_embedding(K::AnticNumberField, c::acb)
    @ Hecke ~/.julia/packages/Hecke/78SWd/src/NumField/ComplexEmbeddings/NfAbs.jl:283
  [4] load_object(s::Oscar.DeserializerState, #unused#::Type{Hecke.NumFieldEmbNfAbs}, dict::Dict{Symbol, Any})
    @ Oscar ~/.julia/dev/Oscar/src/Serialization/Fields.jl:455
  [5] load_typed_object(s::Oscar.DeserializerState, dict::Dict{Symbol, Any}; override_params::Nothing)
    @ Oscar ~/.julia/dev/Oscar/src/Serialization/main.jl:312
  [6] load_typed_object
    @ ~/.julia/dev/Oscar/src/Serialization/main.jl:298 [inlined]
  [7] load(io::IOStream; params::Nothing, type::Nothing, serializer_type::Type)
    @ Oscar ~/.julia/dev/Oscar/src/Serialization/main.jl:646
  [8] load
    @ ~/.julia/dev/Oscar/src/Serialization/main.jl:589 [inlined]
  [9] #5572
    @ ~/.julia/dev/Oscar/src/Serialization/main.jl:665 [inlined]
 [10] open(f::Oscar.var"#5572#5573"{Nothing, Nothing}, args::String; kwargs::Base.Pairs{Symbol, Union{}, Tuple{}, NamedTuple{(), Tuple{}}})
    @ Base ./io.jl:395
 [11] open
    @ ./io.jl:392 [inlined]
 [12] #load#5571
    @ ~/.julia/dev/Oscar/src/Serialization/main.jl:664 [inlined]
 [13] load(filename::String)
    @ Oscar ~/.julia/dev/Oscar/src/Serialization/main.jl:663
 [14] top-level scope
    @ REPL[9]:1

it seems that the following check is true for the two conjugate non-real embeddings, thus throwing an error:
https://github.com/thofma/Hecke.jl/blob/d05f266efbfb605fcd8bc1f7b718576b373a75e0/src/NumField/ComplexEmbeddings/NfAbs.jl#L262

[thofma]

Thanks, I will have a look. I think I know what the problem is.

[thofma]

Fix is on the way. I will ping you once you can try again.

[thofma]

I pushed the fix. Please try again.

[alexej-jordan]

Everything works now, thank you again! I pushed the new serialized data and hopefully CI tests will succeed now.