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

Connect our serialization with Julia serialization #2837

Merged
merged 7 commits into from
Oct 2, 2023

Conversation

antonydellavecchia
Copy link
Collaborator

Minimal working example of distributed processing in oscar.
Needs thofma/Hecke.jl#1219 before merge due to conflicting
serialize methods.

Extends serialize for Oscar types that aren't julia types. Introduces two Oscar serializer types
The JSONSerializer is how the serialization in Oscar has ben used so far. The IPCSerializer
aims at being efficient by not sending the refs, which then leaves it up to the user
to guarantee that the appropriate refs are sent to processes that need them.

See included test for an example.

@antonydellavecchia
Copy link
Collaborator Author

antonydellavecchia commented Sep 21, 2023

This was merged in Hecke thofma/Hecke.jl#1219
and the tests require a Hecke release to pass

@thofma thofma closed this Sep 21, 2023
@thofma thofma reopened this Sep 21, 2023
@thofma
Copy link
Collaborator

thofma commented Sep 21, 2023

There is already a new version. I have restarted CI.

Copy link
Member

@fingolfin fingolfin left a comment

Choose a reason for hiding this comment

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

It seemed OK on a quick look, let's merge and refine later

@fingolfin fingolfin closed this Oct 2, 2023
@fingolfin fingolfin reopened this Oct 2, 2023
@fingolfin fingolfin changed the title Adv/julia serialize Connect our serialization with Julia serialization Oct 2, 2023
@fingolfin fingolfin enabled auto-merge (squash) October 2, 2023 17:04
@codecov
Copy link

codecov bot commented Oct 2, 2023

Codecov Report

Merging #2837 (587bd67) into master (081c60c) will increase coverage by 0.06%.
Report is 30 commits behind head on master.
The diff coverage is 98.43%.

@@            Coverage Diff             @@
##           master    #2837      +/-   ##
==========================================
+ Coverage   80.62%   80.68%   +0.06%     
==========================================
  Files         456      456              
  Lines       64703    69619    +4916     
==========================================
+ Hits        52166    56175    +4009     
- Misses      12537    13444     +907     
Files Coverage Δ
src/Serialization/serializers.jl 100.00% <100.00%> (ø)
src/Serialization/main.jl 87.81% <98.07%> (+0.38%) ⬆️

... and 55 files with indirect coverage changes

@fingolfin fingolfin merged commit aa9dbc2 into master Oct 2, 2023
22 of 29 checks passed
@fingolfin fingolfin deleted the adv/julia-serialize branch October 2, 2023 18:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants