Skip to content

[Merged by Bors] - refactor: redefine ProperCone as an abbrev for ClosedSubmodule#25204

Closed
YaelDillies wants to merge 9 commits intomasterfrom
proper_cone_closed_submodule
Closed

[Merged by Bors] - refactor: redefine ProperCone as an abbrev for ClosedSubmodule#25204
YaelDillies wants to merge 9 commits intomasterfrom
proper_cone_closed_submodule

Conversation

@YaelDillies
Copy link
Copy Markdown
Contributor

... and derive the ProperCone operations from ClosedSubmodule too. Also rename the scalar ring from 𝕜 to R.

From Toric


Open in Gitpod

@YaelDillies YaelDillies added t-analysis Analysis (normed *, calculus) toric Part of the ongoing formalisation of toric varieties labels May 26, 2025
@github-actions
Copy link
Copy Markdown

github-actions bot commented May 26, 2025

PR summary 8024e6939a

Import changes for modified files

Dependency changes

File Base Count Head Count Change
Mathlib.Analysis.Convex.Cone.Basic 1601 1277 -324 (-20.24%)
Mathlib.Analysis.Convex.Cone.InnerDual 1993 1996 +3 (+0.15%)
Import changes for all files
Files Import difference
Mathlib.Analysis.Convex.Cone.Basic -324
Mathlib.Analysis.Convex.Cone.InnerDual 3

Declarations diff

+ coe_bot
+ instance : Coe (ProperCone R E) (PointedCone R E) := ⟨toPointedCone⟩
+ instance : SetLike (ProperCone R E) E
+ mem_bot
+ mem_toPointedCone
+ pointed_toConvexCone
+ smul_mem
+ toPointedCone_bot
+ toPointedCone_positive
- coe_positive
- instZero
- instance : Coe (ProperCone 𝕜 E) (PointedCone 𝕜 E)
- instance : Inhabited (ProperCone 𝕜 E)
- instance : SetLike (ProperCone 𝕜 E) E
- instance : Zero (ProperCone 𝕜 E)

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

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

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


No changes to technical debt.

You can run this locally as

./scripts/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).

Copy link
Copy Markdown
Contributor

@b-mehta b-mehta left a comment

Choose a reason for hiding this comment

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

I don't think it's a good idea to change from k to R in this PR, especially since Analysis/Convex/Cone/Basic uses k everywhere, and the second half of this file does too, so this PR creates a weird situation where R is used in some middle chunk. My suggestion is to make that change file-wide (or folder-wide) in a separate PR.

end T1Space

/-- The closure of image of a proper cone under a `ℝ`-linear map is a proper cone. We
use continuous maps here so that the comap of f is also a map between proper cones. -/
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

If you didn't insist f was continuous, what do you get out instead? I presume just a convex cone or something? (Not an objection, just out of curiosity)

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.

Just a convex cone, yes, and no galois connection

Comment on lines +105 to +109
/-- The closure of image of a proper cone under a linear map is a proper cone.

We use continuous maps here to match `ProperCone.comap`. -/
abbrev map (f : E →L[R] F) (C : ProperCone R E) : ProperCone R F :=
ClosedSubmodule.map (f.restrictScalars R≥0) C
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I don't really buy "it matches" as a good enough reason to have the input non-generalised...?
(But "it's how it was before" is a good enough reason, so this is not an objection :) )

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.

Yeah, just moving this around. I am not convinced myself of the design

@b-mehta b-mehta added the awaiting-author A reviewer has asked the author a question or requested changes. label Jun 10, 2025
@leanprover-community-bot-assistant
Copy link
Copy Markdown
Collaborator

This pull request has conflicts, please merge master and resolve them.

@leanprover-community-bot-assistant leanprover-community-bot-assistant added the merge-conflict The PR has a merge conflict with master, and needs manual merging. (this label is managed by a bot) label Jun 11, 2025
... and derive the `ProperCone` operations from `ClosedSubmodule` too. Also rename the scalar ring from `𝕜` to `R`.

From Toric
@YaelDillies YaelDillies force-pushed the proper_cone_closed_submodule branch from 53bd221 to 53ad1a7 Compare June 11, 2025 10:22
@github-actions github-actions bot added large-import Automatically added label for PRs with a significant increase in transitive imports and removed merge-conflict The PR has a merge conflict with master, and needs manual merging. (this label is managed by a bot) labels Jun 11, 2025
@github-actions github-actions bot removed the large-import Automatically added label for PRs with a significant increase in transitive imports label Jun 11, 2025
@YaelDillies
Copy link
Copy Markdown
Contributor Author

I don't think it's a good idea to change from k to R in this PR, especially since Analysis/Convex/Cone/Basic uses k everywhere, and the second half of this file does too, so this PR creates a weird situation where R is used in some middle chunk. My suggestion is to make that change file-wide (or folder-wide) in a separate PR.

The second part of the file has now been deleted (since we don't have 0 : ClosedSubmodule R M following @erdOne's suggestion here). The file is now consistent in variable use, and the variable rename doesn't impact the diff.

Do you still want me to split the variable rename to another PR?

@YaelDillies YaelDillies removed the awaiting-author A reviewer has asked the author a question or requested changes. label Jun 11, 2025
theorem mem_comap {f : E →L[ℝ] F} {S : ProperCone ℝ F} {x : E} : x ∈ S.comap f ↔ f x ∈ S :=
Iff.rfl
@[simp] lemma mem_positive : x ∈ positive R E ↔ 0 ≤ x := .rfl
@[simp] lemma toPointedCone_positive : (positive R E).toPointedCone = .positive R E := rfl
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Do you want the one about the coercion to ConvexCone too? or does that come for free in simp?

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.

Yes, comes for free in simp since the coercion is obtained by transitivity

@b-mehta
Copy link
Copy Markdown
Contributor

b-mehta commented Jun 12, 2025

Thanks!

bors merge

@ghost ghost added the ready-to-merge This PR has been sent to bors. label Jun 12, 2025
mathlib-bors bot pushed a commit that referenced this pull request Jun 12, 2025
…25204)

... and derive the `ProperCone` operations from `ClosedSubmodule` too. Also rename the scalar ring from `𝕜` to `R`.

From Toric
@mathlib-bors
Copy link
Copy Markdown
Contributor

mathlib-bors bot commented Jun 12, 2025

Pull request successfully merged into master.

Build succeeded:

@mathlib-bors mathlib-bors bot changed the title refactor: redefine ProperCone as an abbrev for ClosedSubmodule [Merged by Bors] - refactor: redefine ProperCone as an abbrev for ClosedSubmodule Jun 12, 2025
@mathlib-bors mathlib-bors bot closed this Jun 12, 2025
@mathlib-bors mathlib-bors bot deleted the proper_cone_closed_submodule branch June 12, 2025 11:25
vlad902 added a commit to vlad902/mathlib4 that referenced this pull request Jun 12, 2025
* master: (447 commits)
  chore(Data/Set): move very basic lemmas earlier (leanprover-community#25707)
  feat: preserve draft status when migrating PRs to fork (leanprover-community#25777)
  chore(Data/Set/Basic): deprecate lemmas that abuse the `Set α := α → Prop` defeq (leanprover-community#25669)
  refactor: redefine `ProperCone` as an `abbrev` for `ClosedSubmodule` (leanprover-community#25204)
  feat: interaction between ContinuousLinearMap.coprod and ContinuousLinearEquiv.prodComm (leanprover-community#25564)
  fix: outdated docstring (leanprover-community#25717)
  feat(MeasureTheory): convolution of measures with densities  (leanprover-community#25718)
  feat: make sure that the simp normal form of equiv-like classes pushes `symm` and `trans` to the right (leanprover-community#25604)
  feat(ModelTheory): Formula.iSup and iInf (leanprover-community#21948)
  feat(RingTheory/Polynomial/ContentIdeal): the content ideal of a polynomial. (leanprover-community#25333)
  fix: typo in labels_from_comment.yml (leanprover-community#25727)
  feat: is{Inducing,Embedding}_prodMkLeft (leanprover-community#25705)
  feat: check that the mathlib options exist (leanprover-community#25687)
  feat(Algebra/Group/Even): Add missing lemmas (leanprover-community#20818)
  refactor: make `Matrix.kroneckerTMulAlgEquiv` heterogeneous (leanprover-community#25693)
  feat: allow contributors to remove labels with comments (leanprover-community#25723)
  chore: disable build.yml and bors.yml on forks (leanprover-community#25722)
  feat: improvements to user_activity_report.py (leanprover-community#25721)
  chore: update the migration script to check for necessary permissions and prefer ssh if possible (leanprover-community#25701)
  chore: update the migration script to check for necessary permissions and prefer ssh if possible (leanprover-community#25701)
  ...
pfaffelh pushed a commit to pfaffelh/mathlib4 that referenced this pull request Jun 15, 2025
…eanprover-community#25204)

... and derive the `ProperCone` operations from `ClosedSubmodule` too. Also rename the scalar ring from `𝕜` to `R`.

From Toric
pfaffelh pushed a commit to pfaffelh/mathlib4 that referenced this pull request Jun 15, 2025
…eanprover-community#25204)

... and derive the `ProperCone` operations from `ClosedSubmodule` too. Also rename the scalar ring from `𝕜` to `R`.

From Toric
callesonne pushed a commit to callesonne/mathlib4 that referenced this pull request Jul 24, 2025
…eanprover-community#25204)

... and derive the `ProperCone` operations from `ClosedSubmodule` too. Also rename the scalar ring from `𝕜` to `R`.

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

Labels

ready-to-merge This PR has been sent to bors. t-analysis Analysis (normed *, calculus) toric Part of the ongoing formalisation of toric varieties

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants