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

[ToricVarieties] Serialize cohomology classes #4266

Merged

Conversation

HereAround
Copy link
Member

This PR aims to serialize cohomology classes on toric varieties. This boils down to saving a polynomial, and ensuring that it resides in the cohomology ring of the toric variety (= a quotient ring)/the base ring of this quotient ring. Sadly I cannot get the code to work. The line:

poly = load_object(s, MPolyDecRingElem, base_ring(cohomology_ring(tv)), :polynomial)

raises the error:

ERROR: KeyError: key Symbol("1") not found

I am out of ideas how to fix this. Any ideas @antonydellavecchia? Help appreciated.

Here is a MWE, based on this PR:

p4 = projective_space(NormalToricVariety, 4)
cc = volume_form(p4)
save("/tmp/test.mrdi", cc)
load("/tmp/test.mrdi")


function save_object(s::SerializerState, cc::CohomologyClass)
save_data_dict(s) do
save_typed_object(s, polynomial(cc).f, :polynomial)
Copy link
Member Author

Choose a reason for hiding this comment

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

Aside, I tried save_typed_object(s, polynomial(cc), :polynomial) first. Unless I am mistaken, serialization for MPolyQuoRingElem is not yet supported, correct?

Copy link
Collaborator

Choose a reason for hiding this comment

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

I think @HechtiDerLachs added support for this? Maybe it hasn't been merged?

Copy link
Member Author

Choose a reason for hiding this comment

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

Ok. But do we really need this here?

The current version of the code tries to serialize a polynomial in the base ring (rather than the quotient ring). In theory, this is sufficient to serialize a (toric) cohomology class. But the loading of said polynomial does not work (yet) - reasons unclear/unknown to me.

@HereAround HereAround marked this pull request as draft November 1, 2024 16:42
@HereAround HereAround marked this pull request as ready for review November 5, 2024 13:30
@HereAround HereAround merged commit 452a1a4 into oscar-system:master Nov 5, 2024
26 checks passed
@HereAround HereAround deleted the SerializeCohomologyClasses branch November 5, 2024 14:28
Copy link

codecov bot commented Nov 5, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 84.49%. Comparing base (4f19d60) to head (456e261).
Report is 9 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #4266      +/-   ##
==========================================
+ Coverage   84.47%   84.49%   +0.02%     
==========================================
  Files         641      641              
  Lines       85394    85459      +65     
==========================================
+ Hits        72133    72210      +77     
+ Misses      13261    13249      -12     
Files with missing lines Coverage Δ
src/Serialization/ToricGeometry.jl 98.41% <100.00%> (+0.37%) ⬆️

... and 4 files with indirect coverage changes

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.

2 participants