-
Notifications
You must be signed in to change notification settings - Fork 126
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
first attempts at writing the serializers for the new types PhylogeneticModel and GroupBasedPhylogeneticModel #4010
base: master
Are you sure you want to change the base?
Conversation
…ticModel and GroupBasedPhylogeneticModel
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #4010 +/- ##
==========================================
+ Coverage 84.58% 84.73% +0.14%
==========================================
Files 596 630 +34
Lines 81969 84510 +2541
==========================================
+ Hits 69336 71606 +2270
- Misses 12633 12904 +271
|
save_object(s, graph(pm), :tree) # already implemented | ||
save_object(s, number_states(pm), :states) # already implemented | ||
save_object(s, probability_ring(pm), :probability_ring) # already implemented | ||
save_object(s, root_distribution(pm), :root_distribution) # needs to be implemented, or we change the entry type to QQFieldElem |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you can using save_typed_object here if the type isn't always know, or if your using a vector any because not all entries are the same type you'll need to use a tuple
src/Serialization/Combinatorics.jl
Outdated
@@ -19,6 +20,19 @@ function load_object(s::DeserializerState, g::Type{Graph{T}}) where T <: Union{D | |||
return g(smallobj) | |||
end | |||
|
|||
function save_object(s::SerializerState, e::Edge) | |||
save_data_dict(s) do |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this can be done using save_data_array instead, it'll make file sizes smaller
…d object Co-authored-by: antonydellavecchia <antonydellavecchia@gmail.com>
…zation of dicts to catch the case where the key is not serialized with params
|
||
|
||
function load_object(s::DeserializerState, g::Type{PhylogeneticModel}) | ||
graph = load_object(s, Graph{Directed}, :tree) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
here you would need to use
graph = load_object(s, Graph{Directed}, :tree) | |
graph = load_typed_object(s, :tree) |
…ests more elaborate. Changed testing for graphs so that false negatives are avoided.
In commit 8723fc9, tests did not pass so I investigated it and found out that apparently object identity does not the same with polymake-derived objects? Therefore, the graph objects of the original model and the loaded one were not identical, although they were the same tree. I wrote a workaround using |
I am currently trying to serialize the new data types introduced in the algebraic statistics (sub)project on algebraic phylogenetics by @marinagarrote et al (cf. #3812). Following the documentation and writing the code as naive as possible, I got problems when testing the functions by trying to save an instance of GroupPhylogeneticModel, with the following error message:
Unsupported type 'GroupBasedPhylogeneticModel' for encoding.