Skip to content

Fix f_vector for cones with lineality (and the point)#5716

Merged
benlorenz merged 11 commits intooscar-system:masterfrom
imkhln:cone_lin_dim_fix
Jan 29, 2026
Merged

Fix f_vector for cones with lineality (and the point)#5716
benlorenz merged 11 commits intooscar-system:masterfrom
imkhln:cone_lin_dim_fix

Conversation

@imkhln
Copy link
Copy Markdown
Contributor

@imkhln imkhln commented Jan 20, 2026

This resolves #5714.

I have no permission to assign but @lkastner this one's for you.

@benlorenz benlorenz requested a review from lkastner January 20, 2026 20:49
Copy link
Copy Markdown
Member

@benlorenz benlorenz left a comment

Choose a reason for hiding this comment

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

Looks like the tests are also wrong, Cone2 should indeed have [1,2] and not [0,2].

Comment on lines +342 to +343
ld = pmc.LINEALITY_DIM
fv = ld == pmc.CONE_DIM ? ZZRingElem[] : pmc.F_VECTOR
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
ld = pmc.LINEALITY_DIM
fv = ld == pmc.CONE_DIM ? ZZRingElem[] : pmc.F_VECTOR
ld = lineality_dim(C)
fv = ld == dim(C) ? ZZRingElem[] : pmc.F_VECTOR::Polymake.Vector{Polymake.Integer}

I recommend adding type assertions for polymake properties because julia cannot deduce the return type and also use the existing methods where possible.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Done.

@lgoettgens lgoettgens changed the title Fix for f_vector(C::Cone) Fix f_vector for cones with lineality Jan 21, 2026
@lgoettgens lgoettgens added topic: polyhedral geometry Issue concerns polyhedral geometry code release notes: use title For PRs: the title of this PR is suitable for direct use in the release notes bug: wrong result labels Jan 21, 2026
@imkhln imkhln closed this Jan 21, 2026
@imkhln imkhln deleted the cone_lin_dim_fix branch January 21, 2026 15:05
@imkhln imkhln reopened this Jan 21, 2026
@imkhln imkhln changed the title Fix f_vector for cones with lineality Fix f_vector for cones with lineality (and the point) Jan 21, 2026
@codecov
Copy link
Copy Markdown

codecov Bot commented Jan 21, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 84.04%. Comparing base (f716867) to head (d30e228).
⚠️ Report is 1 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #5716      +/-   ##
==========================================
- Coverage   84.05%   84.04%   -0.01%     
==========================================
  Files         750      750              
  Lines      103019   102980      -39     
==========================================
- Hits        86591    86554      -37     
+ Misses      16428    16426       -2     
Files with missing lines Coverage Δ
src/PolyhedralGeometry/Cone/properties.jl 90.10% <100.00%> (+0.57%) ⬆️

... and 9 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@benlorenz benlorenz enabled auto-merge (squash) January 29, 2026 13:36
@benlorenz benlorenz merged commit af7cb7f into oscar-system:master Jan 29, 2026
35 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug: wrong result release notes: use title For PRs: the title of this PR is suitable for direct use in the release notes topic: polyhedral geometry Issue concerns polyhedral geometry code

Projects

None yet

Development

Successfully merging this pull request may close these issues.

f-vector of cone with lineality space computed incorrectly

3 participants