Skip to content

n_cones and cones now properly consider all cones#5217

Merged
benlorenz merged 3 commits intomasterfrom
bl/ncones
Aug 21, 2025
Merged

n_cones and cones now properly consider all cones#5217
benlorenz merged 3 commits intomasterfrom
bl/ncones

Conversation

@benlorenz
Copy link
Copy Markdown
Member

With a keyword argument to suppress the trivial cone.

This is slightly breaking but I consider this a bugfix:

The original docstring for cones claimed

Return the ray indices of all non-zero-dimensional cones in a polyhedral fan.

which was not true since it always excluded the minimal cone even when it has a non-empty lineality space (and thus positive dimension).

For ncones:

Return the number of cones of PF.

But this also excluded the trivial cone.

Closes #4815

@benlorenz benlorenz 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 labels Aug 20, 2025
@codecov
Copy link
Copy Markdown

codecov Bot commented Aug 20, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 84.84%. Comparing base (57b958d) to head (128b24f).
⚠️ Report is 9 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #5217      +/-   ##
==========================================
- Coverage   84.84%   84.84%   -0.01%     
==========================================
  Files         710      710              
  Lines       95637    95651      +14     
==========================================
+ Hits        81145    81156      +11     
- Misses      14492    14495       +3     
Files with missing lines Coverage Δ
...try/ToricVarieties/AlgebraicCycles/constructors.jl 86.31% <100.00%> (ø)
...ricVarieties/AlgebraicCycles/special_attributes.jl 100.00% <100.00%> (ø)
src/PolyhedralGeometry/PolyhedralFan/properties.jl 98.50% <100.00%> (+0.02%) ⬆️

... and 3 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 requested a review from lkastner August 21, 2025 09:15
@benlorenz benlorenz merged commit bf99b15 into master Aug 21, 2025
34 checks passed
@benlorenz benlorenz deleted the bl/ncones branch August 21, 2025 14:02
@benlorenz benlorenz changed the title PolyhedralGeometry: (n_)cones now properly consider all cones (n_)cones now properly consider all cones Aug 21, 2025
@lgoettgens lgoettgens changed the title (n_)cones now properly consider all cones (n_)cones now properly considers all cones Aug 26, 2025
@fingolfin fingolfin changed the title (n_)cones now properly considers all cones n_cones and cones now properly consider all cones Sep 11, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

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.

cones and n_cones fail for polyhedral fans consisting only of a lineality space

2 participants