Skip to content

refactor(Algebra/Order): unbundle group and ring cone#37298

Open
artie2000 wants to merge 7 commits intoleanprover-community:masterfrom
artie2000:unbundle-cone
Open

refactor(Algebra/Order): unbundle group and ring cone#37298
artie2000 wants to merge 7 commits intoleanprover-community:masterfrom
artie2000:unbundle-cone

Conversation

@artie2000
Copy link
Copy Markdown
Collaborator

@artie2000 artie2000 commented Mar 28, 2026

  • Unbundle GroupCone and RingCone using Submonoid.IsMulPointed

The material in Mathlib.Algebra.Group.Submonoid.Support was created to treat uniformly

  • positive cones in groups and rings
  • pointed cones in vector spaces over an ordered field
  • orderings in rings

This PR deprecates the GroupCone and RingCone structures, making use of the predicates Submonoid.IsMulPointed and AddSubmonoid.IsPointed defined in that file instead.

See also #36863 for the analogous change to ring orderings.


Open in Gitpod

@github-actions github-actions bot added the large-import Automatically added label for PRs with a significant increase in transitive imports label Mar 28, 2026
@github-actions
Copy link
Copy Markdown

github-actions bot commented Mar 28, 2026

PR summary 35471d41e9

Import changes exceeding 2%

% File
+89.37% Mathlib.Algebra.Order.Group.Cone
+67.15% Mathlib.Algebra.Order.Ring.Cone

Import changes for modified files

Dependency changes

File Base Count Head Count Change
Mathlib.Algebra.Order.Group.Cone 348 659 +311 (+89.37%)
Mathlib.Algebra.Order.Ring.Cone 414 692 +278 (+67.15%)
Import changes for all files
Files Import difference
Mathlib.Algebra.Order.Ring.Cone 278
Mathlib.Algebra.Order.Group.Cone 311

Declarations diff

+ IsOrderedMonoid.mkOfSubmonoid
+ IsOrderedRing.mkOfSubsemiring
+ LinearOrder.mkOfSubmonoid
+ PartialOrder.mkOfSubmonoid
+ PartialOrder.mkOfSubmonoid_le_iff
+ Submonoid.oneLE.isMulPointed
+ Submonoid.oneLE.isMulSpanning
+ Subsemiring.nonneg.isPointed
+ Subsemiring.nonneg.isSpanning

You can run this locally as follows
## summary with just the declaration names:
./scripts/pr_summary/declarations_diff.sh <optional_commit>

## more verbose report:
./scripts/pr_summary/declarations_diff.sh long <optional_commit>

The doc-module for scripts/pr_summary/declarations_diff.sh contains some details about this script.


Increase in tech debt: (relative, absolute) = (3.00, 0.12)
Current number Change Type
25 3 disabled deprecation lints

Current commit 85c2dc228d
Reference commit 35471d41e9

You can run this locally as

./scripts/reporting/technical-debt-metrics.sh pr_summary
  • The relative value is the weighted sum of the differences with weight given by the inverse of the current value of the statistic.
  • The absolute value is the relative value divided by the total sum of the inverses of the current values (i.e. the weighted average of the differences).

@github-actions github-actions bot added the t-algebra Algebra (groups, rings, fields, etc) label Mar 28, 2026
@dagurtomas dagurtomas removed their assignment Apr 2, 2026
@joelriou joelriou removed their assignment Apr 6, 2026
@joneugster
Copy link
Copy Markdown
Contributor

Could you please extend the PR description explaining why this is desired or what thee benefits are? And if there is a relevant Zulip discussion, could you please link that, too?

Not sure I'm the right person to review. I'll unassign myself for now but keep this on my list to revisit again

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

large-import Automatically added label for PRs with a significant increase in transitive imports t-algebra Algebra (groups, rings, fields, etc)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants