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.

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

Serialize Types with Attributes #4020

Merged
merged 30 commits into from
Aug 22, 2024
Merged
Show file tree
Hide file tree
Changes from 20 commits
Commits
Show all changes
30 commits
Select commit Hold shift + click to select a range
bf3f0b2
first attempts at writing the serializers for the new types Phylogene…
mariusneu Aug 13, 2024
7f08d36
include serialization functions from AlgebraicStatistics
antonydellavecchia Aug 14, 2024
efc86ef
Merge remote-tracking branch 'origin/master'
antonydellavecchia Aug 15, 2024
f0031f0
first steps to serializing attributes
antonydellavecchia Aug 15, 2024
8ea00cb
saving and loading attributes working
antonydellavecchia Aug 16, 2024
98a8c2a
added some documentation on with_attrs
antonydellavecchia Aug 19, 2024
27e8a78
updated docs, + removed algebraic stats commits, and fixed comment
antonydellavecchia Aug 19, 2024
a388ce9
Update experimental/AlgebraicStatistics/src/AlgebraicStatistics.jl
antonydellavecchia Aug 19, 2024
f2f3a89
Apply suggestions from code review
antonydellavecchia Aug 19, 2024
8eda5d3
Merge branch 'master' into adv/serialize-attrs
lgoettgens Aug 19, 2024
5eeaf4e
includes load_attrs and save_attrs in import all function
antonydellavecchia Aug 19, 2024
f80dd74
Apply suggestions from code review
antonydellavecchia Aug 19, 2024
5a848c1
changed type_attr_map to a function
antonydellavecchia Aug 20, 2024
513d32b
type_attr_map -> type_attr_dict
antonydellavecchia Aug 20, 2024
a55f53f
uses encode type to setup type_attr_map
antonydellavecchia Aug 20, 2024
d1984fa
still issues with the test
antonydellavecchia Aug 20, 2024
fbef097
More `Type` -> `String`
lgoettgens Aug 20, 2024
4113c4c
Merge remote-tracking branch 'origin/adv/serialize-attrs' into adv/se…
antonydellavecchia Aug 20, 2024
bfb0f29
forgot to with_attrs to save in setup_tests
antonydellavecchia Aug 20, 2024
e243868
toric tests passing
antonydellavecchia Aug 20, 2024
da71299
moved save_attrs to appropriate line
antonydellavecchia Aug 20, 2024
29e9e03
moved attrs_list to appropriate line
antonydellavecchia Aug 20, 2024
9732fb7
Update src/Serialization/main.jl
antonydellavecchia Aug 20, 2024
8b42ba7
Update src/Serialization/main.jl
antonydellavecchia Aug 20, 2024
d8cf5d3
Update src/Serialization/serializers.jl
antonydellavecchia Aug 20, 2024
83cd038
Update src/Serialization/main.jl
antonydellavecchia Aug 20, 2024
5e763d5
small fix still need to move the check
antonydellavecchia Aug 20, 2024
e47bca0
added check inside save_attrs
antonydellavecchia Aug 22, 2024
da15cbe
offline review
antonydellavecchia Aug 22, 2024
548ab18
fixed save docs
antonydellavecchia Aug 22, 2024
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
23 changes: 20 additions & 3 deletions src/Serialization/ToricGeometry.jl
Original file line number Diff line number Diff line change
@@ -1,13 +1,30 @@
################################################################################
# Toric varieties
@register_serialization_type AffineNormalToricVariety uses_id
@register_serialization_type NormalToricVariety uses_id

function save_object(s::SerializerState, ntv::NormalToricVarietyType)
save_object(s, ntv.polymakeNTV)
@register_serialization_type NormalToricVariety uses_id [:cox_ring]


function save_object(s::SerializerState, ntv::T) where T <: NormalToricVarietyType
attrs = attrs_list(s, T)

if !isnothing(attrs) && any([has_attribute(ntv, attr) for attr in attrs])
Copy link
Member

Choose a reason for hiding this comment

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

attrs will never be nothing, I think you need isempty for what you had here earlier.

you could, in theory, just save NTV from now on always as a dicts with pm_data and attrs, and only handle the old format in loading. this would make the saving already simpler, and it would not double the check for the presence of any attribute with https://github.com/oscar-system/Oscar.jl/pull/4020/files#r1723508786

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

yes, I think the check here instead makes sense, this would then altert he toric variety files and this will introduce a breaking change, i'll need to write an upgrade script + probably upgrade martin's artifacts for f theory.

Copy link
Member

Choose a reason for hiding this comment

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

cf #4020 (comment)
tldr: I am not involved in NTV, so feel free to ignore my comments about that and instead talk to the people having a say there :)

save_data_dict(s) do
save_attrs(s, ntv)
save_object(s, ntv.polymakeNTV, :pm_data)
end
else
save_object(s, ntv.polymakeNTV)
end
antonydellavecchia marked this conversation as resolved.
Show resolved Hide resolved
end

function load_object(s::DeserializerState, ::Type{T}) where {T <: Union{NormalToricVariety, AffineNormalToricVariety}}
if haskey(s, :pm_data)
ntv = T(load_object(s, Polymake.BigObject, :pm_data))
load_attrs(s, ntv)

return ntv
end
return T(load_object(s, Polymake.BigObject))
end

Expand Down
92 changes: 72 additions & 20 deletions src/Serialization/main.jl
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,10 @@ function get_oscar_serialization_version()
return oscar_serialization_version[] = result
end

################################################################################
# Type attribute map
const type_attr_map = Dict{String, Vector{Symbol}}()

################################################################################
# (De|En)coding types

Expand Down Expand Up @@ -189,6 +193,14 @@ function save_type_params(s::SerializerState, obj::Any, key::Symbol)
save_type_params(s, obj)
end

function save_attrs(s::SerializerState, obj::T) where T
antonydellavecchia marked this conversation as resolved.
Show resolved Hide resolved
save_data_dict(s, :attrs) do
for attr in attrs_list(s, T)
has_attribute(obj, attr) && save_typed_object(s, get_attribute(obj, attr), attr)
end
end
Copy link
Member

Choose a reason for hiding this comment

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

this, currently, always saves a dict, which may often be empty. In particular, this also gets saved when passing with_attrs=false. To circumvent this, you could add something like the following:

Suggested change
save_data_dict(s, :attrs) do
for attr in attrs_list(s, T)
has_attribute(obj, attr) && save_typed_object(s, get_attribute(obj, attr), attr)
end
end
if any(attr -> has_attribute(obj, attr), attrs_list(s, T))
save_data_dict(s, :attrs) do
for attr in attrs_list(s, T)
has_attribute(obj, attr) && save_typed_object(s, get_attribute(obj, attr), attr)
end
end
end

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

ill add the check here to not block lie algebra serialization, but also keep the check in toric varieties for now, and see if other similar examples popup and adjust them all at the same time.

Copy link
Member

Choose a reason for hiding this comment

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

Great, thanks!

end

# The load mechanism first checks if the type needs to load necessary
# parameters before loading it's data, if so a type tree is traversed
function load_typed_object(s::DeserializerState, key::Symbol; override_params::Any = nothing)
Expand Down Expand Up @@ -239,6 +251,14 @@ function load_object(s::DeserializerState, T::Type, params::Any, key::Union{Symb
end
end

function load_attrs(s::DeserializerState, obj::T) where T
load_node(s, :attrs) do _
antonydellavecchia marked this conversation as resolved.
Show resolved Hide resolved
for attr in attrs_list(s, T)
haskey(s, attr) && set_attribute!(obj, attr, load_typed_object(s, attr))
end
end
end

################################################################################
# Default generic save_internal, load_internal
function save_object_generic(s::SerializerState, obj::T) where T
Expand Down Expand Up @@ -283,6 +303,13 @@ function register_serialization_type(@nospecialize(T::Type), str::String)
reverse_type_map[str] = T
end

function register_attr_list(@nospecialize(T::Type),
attrs::Union{Vector{Symbol}, Nothing})
if !isnothing(attrs)
Oscar.type_attr_map[encode_type(T)] = attrs
end
end

import Serialization.serialize
import Serialization.deserialize
import Serialization.serialize_type
Expand All @@ -295,7 +322,8 @@ serialize_with_id(obj::Any) = false
serialize_with_params(::Type) = false


function register_serialization_type(ex::Any, str::String, uses_id::Bool, uses_params::Bool)
function register_serialization_type(ex::Any, str::String, uses_id::Bool,
uses_params::Bool, attrs::Any)
return esc(
quote
Oscar.register_serialization_type($ex, $str)
Expand All @@ -313,6 +341,9 @@ function register_serialization_type(ex::Any, str::String, uses_id::Bool, uses_p
# Types like ZZ, QQ, and ZZ/nZZ do not require ids since there is no syntactic
# ambiguities in their encodings.

# add list of possible attributes to save for a given type to a global dict
Oscar.register_attr_list($ex, $attrs)

Oscar.serialize_with_id(obj::T) where T <: $ex = $uses_id
Oscar.serialize_with_id(T::Type{<:$ex}) = $uses_id
Oscar.serialize_with_params(T::Type{<:$ex}) = $uses_params
Expand All @@ -327,45 +358,52 @@ function register_serialization_type(ex::Any, str::String, uses_id::Bool, uses_p
Oscar.load(s.io; serializer_type=Oscar.IPCSerializer)
end
end

end)
end

"""
@register_serialization_type NewType "String Representation of type" uses_id uses_params
@register_serialization_type NewType "String Representation of type" uses_id uses_params [:attr1, :attr2]

`@register_serialization_type` is a macro to ensure that the string we generate
matches exactly the expression passed as first argument, and does not change
in unexpected ways when import/export statements are adjusted.

The last three arguments are optional and can arise in any order.
Passing a string argument will override how the type is stored as a string.
The last two are boolean flags. When setting `uses_id` the object will be
stored as a reference and will be referred to throughout the serialization
sessions using a `UUID`. This should typically only be used for types that
do not have a fixed normal form for example `PolyRing` and `MPolyRing`.

When setting `uses_id` the object will be stored as a reference and
will be referred to throughout the serialization sessions using a `UUID`.
This should typically only be used for types that do not have a fixed
normal form for example `PolyRing` and `MPolyRing`.

Using the `uses_params` flag will serialize the object with a more structured type
description which will make the serialization more efficient see the discussion on
`save_type_params` / `load_type_params` below.

Passing a vector of symbols that correspond to attributes of type
indicates which attributes will be serialized when using save with `with_attrs=true`.

"""
macro register_serialization_type(ex::Any, args...)
uses_id = false
uses_params = false
str = nothing
attrs = nothing
for el in args
if el isa String
str = el
elseif el == :uses_id
uses_id = true
elseif el == :uses_params
uses_params = true
else
attrs = el
end
end
if str === nothing
str = string(ex)
end

return register_serialization_type(ex, str, uses_id, uses_params)
return register_serialization_type(ex, str, uses_id, uses_params, attrs)
end


Expand Down Expand Up @@ -394,11 +432,13 @@ macro import_all_serialization_functions()
encode_type,
haskey,
load_array_node,
load_attrs,
load_node,
load_params_node,
load_ref,
load_typed_object,
save_as_ref,
save_attrs,
save_data_array,
save_data_basic,
save_data_dict,
Expand Down Expand Up @@ -435,11 +475,14 @@ include("Upgrades/main.jl")
# Interacting with IO streams and files

"""
save(io::IO, obj::Any; metadata::MetaData=nothing)
save(filename::String, obj::Any, metadata::MetaData=nothing)
save(io::IO, obj::Any; metadata::MetaData=nothing, with_attrs::Bool=false)
save(filename::String, obj::Any, metadata::MetaData=nothing, with_attrs::Bool=false)
Copy link
Member

Choose a reason for hiding this comment

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

This now longer matches the changed default

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

thanks!


Save an object `obj` to the given io stream
respectively to the file `filename`.
respectively to the file `filename`. When used with `with_attrs=true` then the object will
save it's attributes along with all the attributes of the types used in the object's struct.
The attributes that will be saved are defined during type registration see
[`@register_serialization_type`](@ref)

See [`load`](@ref).

Expand All @@ -463,8 +506,11 @@ julia> load("/tmp/fourtitwo.mrdi")
```
"""
function save(io::IO, obj::T; metadata::Union{MetaData, Nothing}=nothing,
with_attrs::Bool=false,
serializer_type::Type{<: OscarSerializer} = JSONSerializer) where T
s = state(serializer_open(io, serializer_type))

s = state(serializer_open(io, serializer_type,
with_attrs ? type_attr_map : Dict{String, Vector{Symbol}}()))
save_data_dict(s) do
# write out the namespace first
save_header(s, get_oscar_serialization_version(), :_ns)
Expand Down Expand Up @@ -502,20 +548,21 @@ function save(io::IO, obj::T; metadata::Union{MetaData, Nothing}=nothing,
return nothing
end

function save(filename::String, obj::Any; metadata::Union{MetaData, Nothing}=nothing)
function save(filename::String, obj::Any; metadata::Union{MetaData, Nothing}=nothing,
with_attrs::Bool=false)
dir_name = dirname(filename)
# julia dirname does not return "." for plain filenames without any slashes
temp_file = tempname(isempty(dir_name) ? pwd() : dir_name)
open(temp_file, "w") do file
save(file, obj; metadata=metadata)
save(file, obj; metadata=metadata, with_attrs=with_attrs)
end
Base.Filesystem.rename(temp_file, filename) # atomic "multi process safe"
return nothing
end

"""
load(io::IO; params::Any = nothing, type::Any = nothing)
load(filename::String; params::Any = nothing, type::Any = nothing)
load(io::IO; params::Any = nothing, type::Any = nothing, with_attrs::Bool=false)
load(filename::String; params::Any = nothing, type::Any = nothing, with_attrs::Bool=false)
antonydellavecchia marked this conversation as resolved.
Show resolved Hide resolved

Load the object stored in the given io stream
respectively in the file `filename`.
Expand All @@ -529,6 +576,9 @@ results in setting its parent, or in the case of a container of ring types such
If a type `T` is given then attempt to load the root object of the data
being loaded with this type; if this fails, an error is thrown.

If `with_attrs=true` the object will be loaded with attributes available from
the file (or serialized data).

See [`save`](@ref).

# Examples
Expand Down Expand Up @@ -568,8 +618,9 @@ true
```
"""
function load(io::IO; params::Any = nothing, type::Any = nothing,
serializer_type=JSONSerializer)
s = state(deserializer_open(io, serializer_type))
serializer_type=JSONSerializer, with_attrs::Bool=true)
s = state(deserializer_open(io, serializer_type,
with_attrs ? type_attr_map : Dict{String, Vector{Symbol}}()))
if haskey(s.obj, :id)
id = s.obj[:id]
if haskey(global_serializer_state.id_to_obj, UUID(id))
Expand Down Expand Up @@ -658,7 +709,8 @@ function load(io::IO; params::Any = nothing, type::Any = nothing,
end
end

function load(filename::String; params::Any = nothing, type::Any = nothing)
function load(filename::String; params::Any = nothing,
type::Any = nothing, with_attrs::Bool=false)
antonydellavecchia marked this conversation as resolved.
Show resolved Hide resolved
open(filename) do file
return load(file; params=params, type=type)
end
Expand Down
31 changes: 24 additions & 7 deletions src/Serialization/serializers.jl
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,11 @@ mutable struct SerializerState
refs::Vector{UUID}
io::IO
key::Union{Symbol, Nothing}
type_attr_map::Dict{String, Vector{Symbol}}
end

function attrs_list(s::Union{SerializerState, DeserializerState}, T::Type)
return get(s.type_attr_map, encode_type(T), Symbol[])
end
lgoettgens marked this conversation as resolved.
Show resolved Hide resolved

function begin_node(s::SerializerState)
Expand Down Expand Up @@ -142,6 +147,7 @@ mutable struct DeserializerState
obj::Union{Dict{Symbol, Any}, Vector, JSON3.Object, JSON3.Array, BasicTypeUnion}
key::Union{Symbol, Int, Nothing}
refs::Union{Dict{Symbol, Any}, JSON3.Object, Nothing}
type_attr_map::Dict{String, Vector{Symbol}}
end

# general loading of a reference
Expand Down Expand Up @@ -212,26 +218,37 @@ end

state(s::OscarSerializer) = s.state

function serializer_open(io::IO, T::Type{<: OscarSerializer})
function serializer_open(
io::IO,
T::Type{<: OscarSerializer},
type_attr_map::S) where S <: Union{Dict{String, Vector{Symbol}}, Nothing}

# some level of handling should be done here at a later date
return T(SerializerState(true, UUID[], io, nothing))
return T(SerializerState(true, UUID[], io, nothing, type_attr_map))
end

function deserializer_open(io::IO, T::Type{JSONSerializer})
function deserializer_open(
io::IO,
T::Type{JSONSerializer},
type_attr_map::S) where S <:Union{Dict{String, Vector{Symbol}}, Nothing}

obj = JSON3.read(io)
refs = nothing
if haskey(obj, refs_key)
refs = obj[refs_key]
end

return T(DeserializerState(obj, nothing, refs))
return T(DeserializerState(obj, nothing, refs, type_attr_map))
end

function deserializer_open(io::IO, T::Type{IPCSerializer})
function deserializer_open(
io::IO,
T::Type{IPCSerializer},
type_attr_map::S) where S <:Union{Dict{String, Vector{Symbol}}, Nothing}
# Using a JSON3.Object from JSON3 version 1.13.2 causes
# @everywhere using Oscar
# to hang. So we use a Dict here for now.

obj = JSON.parse(io, dicttype=Dict{Symbol, Any})
return T(DeserializerState(obj, nothing, nothing))
return T(DeserializerState(obj, nothing, nothing, type_attr_map))
end
5 changes: 4 additions & 1 deletion test/Serialization/ToricGeometry.jl
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,13 @@
mktempdir() do path
@testset "NormalToricVariety" begin
pp = projective_space(NormalToricVariety, 2)
test_save_load_roundtrip(path, pp) do loaded
R = cox_ring(pp)
loaded_from_empty_state = test_save_load_roundtrip(path, pp; with_attrs=true) do loaded
@test rays(pp) == rays(loaded)
@test ray_indices(maximal_cones(pp)) == ray_indices(maximal_cones(loaded))
end

@test has_attribute(loaded_from_empty_state, :cox_ring)
end

@testset "ToricDivisor" begin
Expand Down
Loading
Loading