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

updates related to polymake 4.12 #3819

Merged
merged 9 commits into from
Jul 22, 2024
Merged

updates related to polymake 4.12 #3819

merged 9 commits into from
Jul 22, 2024

Conversation

benlorenz
Copy link
Member

@benlorenz benlorenz commented Jun 3, 2024

  • rounding for OscarNumber
  • fix for graph edge iterators
  • add some type mapping for nodemaps for polymake posets
  • delayed loading for graphmaps when loading polymake objects from dicts

todo:

Copy link

codecov bot commented Jun 3, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 84.08%. Comparing base (5f6b05e) to head (f28ad4f).
Report is 25 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #3819      +/-   ##
==========================================
+ Coverage   83.96%   84.08%   +0.11%     
==========================================
  Files         591      592       +1     
  Lines       81384    81775     +391     
==========================================
+ Hits        68335    68759     +424     
+ Misses      13049    13016      -33     
Files Coverage Δ
src/Combinatorics/Graphs/functions.jl 95.32% <100.00%> (+0.04%) ⬆️
src/PolyhedralGeometry/Polyhedron/properties.jl 86.50% <100.00%> (ø)
src/PolyhedralGeometry/helpers.jl 82.05% <100.00%> (+0.57%) ⬆️
src/Serialization/polymake.jl 75.24% <100.00%> (+5.58%) ⬆️

... and 32 files with indirect coverage changes

@fingolfin fingolfin mentioned this pull request Jun 6, 2024
@benlorenz benlorenz changed the title updates related to polymake 4.12 polymake posets and updates related to polymake 4.12 Jul 11, 2024
@lgoettgens
Copy link
Member

I think it would make sense to split this PR into two: one regarding the small fixes for polymake 4.12 (which seems to be almost finished) and a separate one for the post functionality.
This would make it easier for other people interested in posets (like e.g. @felix-roehrich) to take part in a discussion there, without holding back the other changes unnecessarily long.
(and that's how we do it with the other cornerstones usually as well, i.e. don't mix new features and adaptions to an update too much to keep everything transparent and comprehensible.)

@benlorenz
Copy link
Member Author

benlorenz commented Jul 11, 2024

I think it would make sense to split this PR into two: one regarding the small fixes for polymake 4.12 (which seems to be almost finished) and a separate one for the post functionality. This would make it easier for other people interested in posets (like e.g. @felix-roehrich) to take part in a discussion there, without holding back the other changes unnecessarily long. (and that's how we do it with the other cornerstones usually as well, i.e. don't mix new features and adaptions to an update too much to keep everything transparent and comprehensible.)

That makes sense, I will split this up.
Edit: done in #3928

@benlorenz benlorenz mentioned this pull request Jul 11, 2024
@benlorenz benlorenz changed the title polymake posets and updates related to polymake 4.12 updates related to polymake 4.12 Jul 11, 2024
@benlorenz benlorenz marked this pull request as ready for review July 11, 2024 10:14
@lgoettgens
Copy link
Member

The error in CI seems like something real: https://github.com/oscar-system/Oscar.jl/actions/runs/9927077065/job/27421571688?pr=3819#step:9:3937
However, it only occurs on macOS.

@benlorenz
Copy link
Member Author

The error in CI seems like something real: oscar-system/Oscar.jl/actions/runs/9927077065/job/27421571688?pr=3819#step:9:3937 However, it only occurs on macOS.

Yes, I am aware of that, there are some extra conversion helpers missing for Pair/Tuple types, I need to add them in Polymake.jl. (The reason is that on Linux CxxLong is just an alias for Int64, but on macOS CxxLong is a separate type, and then some default constructions don't match as expected)

@benlorenz benlorenz marked this pull request as draft July 14, 2024 12:48
@benlorenz benlorenz closed this Jul 19, 2024
@benlorenz benlorenz reopened this Jul 19, 2024
@benlorenz benlorenz marked this pull request as ready for review July 19, 2024 19:00
@benlorenz benlorenz merged commit e548b0d into master Jul 22, 2024
33 of 57 checks passed
@benlorenz benlorenz deleted the bl/decoration branch July 22, 2024 14:40
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