-
Notifications
You must be signed in to change notification settings - Fork 995
[Merged by Bors] - feat: add ContinuousSMul instances for ℚ≥0
#28474
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
[Merged by Bors] - feat: add ContinuousSMul instances for ℚ≥0
#28474
Conversation
astrainfinita
commented
Aug 15, 2025
|
!bench |
PR summary 766e19e781
|
| File | Base Count | Head Count | Change |
|---|---|---|---|
| Mathlib.Topology.Instances.Rat | 1195 | 1197 | +2 (+0.17%) |
Import changes for all files
| Files | Import difference |
|---|---|
81 filesMathlib.Analysis.LConvolution Mathlib.Analysis.LocallyConvex.BalancedCoreHull Mathlib.Analysis.LocallyConvex.Basic Mathlib.Analysis.Normed.Field.Lemmas Mathlib.Analysis.Normed.Field.ProperSpace Mathlib.Analysis.Normed.Field.UnitBall Mathlib.Analysis.Normed.Group.Rat Mathlib.Analysis.Normed.Order.Lattice Mathlib.Dynamics.Ergodic.Action.Basic Mathlib.Dynamics.Ergodic.Action.Regular Mathlib.Dynamics.Ergodic.Conservative Mathlib.MeasureTheory.Category.MeasCat Mathlib.MeasureTheory.Constructions.BorelSpace.Basic Mathlib.MeasureTheory.Constructions.BorelSpace.Metric Mathlib.MeasureTheory.Constructions.BorelSpace.Metrizable Mathlib.MeasureTheory.Constructions.BorelSpace.Order Mathlib.MeasureTheory.Constructions.BorelSpace.Real Mathlib.MeasureTheory.Constructions.BorelSpace.WithTop Mathlib.MeasureTheory.Constructions.ClosedCompactCylinders Mathlib.MeasureTheory.Constructions.Pi Mathlib.MeasureTheory.Constructions.Polish.Basic Mathlib.MeasureTheory.Constructions.Polish.EmbeddingReal Mathlib.MeasureTheory.Covering.Vitali Mathlib.MeasureTheory.Function.AEMeasurableOrder Mathlib.MeasureTheory.Function.EssSup Mathlib.MeasureTheory.Function.Floor Mathlib.MeasureTheory.Function.SimpleFuncDense Mathlib.MeasureTheory.Function.SimpleFunc Mathlib.MeasureTheory.Group.AEStabilizer Mathlib.MeasureTheory.Group.Action Mathlib.MeasureTheory.Group.Convolution Mathlib.MeasureTheory.Group.LIntegral Mathlib.MeasureTheory.Group.Measure Mathlib.MeasureTheory.Group.Prod Mathlib.MeasureTheory.Integral.Indicator Mathlib.MeasureTheory.Integral.Lebesgue.Add Mathlib.MeasureTheory.Integral.Lebesgue.Basic Mathlib.MeasureTheory.Integral.Lebesgue.Countable Mathlib.MeasureTheory.Integral.Lebesgue.DominatedConvergence Mathlib.MeasureTheory.Integral.Lebesgue.Map Mathlib.MeasureTheory.Integral.Lebesgue.Markov Mathlib.MeasureTheory.Integral.Lebesgue.MeasurePreserving Mathlib.MeasureTheory.Integral.Lebesgue.Norm Mathlib.MeasureTheory.Integral.Lebesgue.Sub Mathlib.MeasureTheory.Integral.Lebesgue Mathlib.MeasureTheory.Integral.Marginal Mathlib.MeasureTheory.Measure.Content Mathlib.MeasureTheory.Measure.ContinuousPreimage Mathlib.MeasureTheory.Measure.GiryMonad Mathlib.MeasureTheory.Measure.Haar.Basic Mathlib.MeasureTheory.Measure.OpenPos Mathlib.MeasureTheory.Measure.Prod Mathlib.MeasureTheory.Measure.Regular Mathlib.MeasureTheory.Measure.RegularityCompacts Mathlib.MeasureTheory.Measure.Stieltjes Mathlib.MeasureTheory.Measure.Support Mathlib.MeasureTheory.Measure.Tight Mathlib.MeasureTheory.Measure.WithDensity Mathlib.NumberTheory.Harmonic.Int Mathlib.NumberTheory.Padics.Hensel Mathlib.NumberTheory.Padics.PadicIntegers Mathlib.NumberTheory.Padics.PadicNumbers Mathlib.Probability.Decision.Risk.Defs Mathlib.Probability.Kernel.Basic Mathlib.Probability.Kernel.Composition.CompMap Mathlib.Probability.Kernel.Composition.CompNotation Mathlib.Probability.Kernel.Composition.CompProd Mathlib.Probability.Kernel.Composition.Comp Mathlib.Probability.Kernel.Composition.KernelLemmas Mathlib.Probability.Kernel.Composition.MapComap Mathlib.Probability.Kernel.Composition.ParallelComp Mathlib.Probability.Kernel.Composition.Prod Mathlib.Probability.Kernel.Defs Mathlib.Probability.Kernel.IonescuTulcea.PartialTraj Mathlib.Probability.Kernel.MeasurableLIntegral Mathlib.Probability.Kernel.Proper Mathlib.RingTheory.DividedPowers.Padic Mathlib.Topology.Baire.BaireMeasurable Mathlib.Topology.Instances.RatLemmas Mathlib.Topology.Instances.RealVectorSpace Mathlib.Topology.UniformSpace.Dini |
1 |
Mathlib.Topology.Instances.Rat Mathlib.Topology.UniformSpace.CompareReals |
2 |
Declarations diff
+ instance : ContinuousSMul ℚ ℝ
+ instance : ContinuousSMul ℚ≥0 NNReal
+ instance {R : Type*} [TopologicalSpace R] [MulAction ℚ R] [MulAction ℚ≥0 R] [IsScalarTower ℚ≥0 ℚ R]
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
relativevalue is the weighted sum of the differences with weight given by the inverse of the current value of the statistic. - The
absolutevalue is therelativevalue divided by the total sum of the inverses of the current values (i.e. the weighted average of the differences).
Mathlib/Topology/Instances/Rat.lean
Outdated
| instance : ContinuousSMul ℚ ℝ where | ||
| continuous_smul := continuous_induced_dom.fst'.smul (M := ℝ) (X := ℝ) continuous_snd | ||
|
|
||
| instance {R} [Ring R] [TopologicalSpace R] [Algebra ℚ R] [Algebra ℚ≥0 R] [ContinuousSMul ℚ R] : |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is the Algebra ℚ≥0 R hypothesis necessary? Why not show it within the proof for continuous_smul?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The argument of ContinuousSMul ℚ≥0 R needs Algebra ℚ≥0 R. Obtaining it from Algebra ℚ R will cause a diamond.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ah, fair enough
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't agree with the claim about the diamond (in fact I claim the opposite!) but it doesn't matter since I have another suggestion.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the diamond already exists. Zulip
Co-authored-by: themathqueen <[email protected]>
Mathlib/Topology/Instances/Rat.lean
Outdated
| instance : ContinuousSMul ℚ ℝ where | ||
| continuous_smul := continuous_induced_dom.fst'.smul (M := ℝ) (X := ℝ) continuous_snd | ||
|
|
||
| instance {R} [Ring R] [TopologicalSpace R] [Algebra ℚ R] [Algebra ℚ≥0 R] [ContinuousSMul ℚ R] : |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't agree with the claim about the diamond (in fact I claim the opposite!) but it doesn't matter since I have another suggestion.
Mathlib/Topology/Instances/Rat.lean
Outdated
| instance {R} [Ring R] [TopologicalSpace R] [Algebra ℚ R] [Algebra ℚ≥0 R] [ContinuousSMul ℚ R] : | ||
| ContinuousSMul ℚ≥0 R where | ||
| continuous_smul := by | ||
| convert continuous_induced_dom.fst'.smul (M := ℚ) (X := R) continuous_snd using 2 | ||
| rw [← cast_smul_eq_nnqsmul ℚ] | ||
| rfl |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note that via NNRat.instDistribMulActionOfRat we already have an action of ℚ≥0 on R so there is no need to include a typeclass carrying this. Also the Algebra assumption is more than we need and can be weakened to just DistribMulAction. Here is what I suggest:
| instance {R} [Ring R] [TopologicalSpace R] [Algebra ℚ R] [Algebra ℚ≥0 R] [ContinuousSMul ℚ R] : | |
| ContinuousSMul ℚ≥0 R where | |
| continuous_smul := by | |
| convert continuous_induced_dom.fst'.smul (M := ℚ) (X := R) continuous_snd using 2 | |
| rw [← cast_smul_eq_nnqsmul ℚ] | |
| rfl | |
| instance {R : Type*} [Ring R] [DistribMulAction ℚ R] [TopologicalSpace R] [ContinuousSMul ℚ R] : | |
| ContinuousSMul ℚ≥0 R where | |
| continuous_smul := continuous_induced_dom.fst'.smul continuous_snd |
|
!bench |
|
Here are the benchmark results for commit 94eb313. |
This fixes the instances diamond in the `NNRat` action.
ocfnash
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've left some suggestions; feel free to revert to your original design if these give trouble.
Thanks!
bors d+
|
✌️ astrainfinita can now approve this pull request. To approve and merge a pull request, simply reply with |
|
bors r+ |
|
Build failed: |
|
I think this failed because of a silly disk space error so let's try again bors merge |
|
Pull request successfully merged into master. Build succeeded: |
ContinuousSMul instances for ℚ≥0ContinuousSMul instances for ℚ≥0