perf(Analysis/Normed/Group): lower instance priorities#37962
perf(Analysis/Normed/Group): lower instance priorities#37962JovanGerb wants to merge 9 commits intoleanprover-community:masterfrom
Conversation
PR summary 3d8100bbc6
|
| File | Base Count | Head Count | Change |
|---|---|---|---|
| Mathlib.Analysis.Complex.Isometry | 1981 | 1982 | +1 (+0.05%) |
Import changes for all files
| Files | Import difference |
|---|---|
Mathlib.Analysis.Complex.Isometry |
1 |
Declarations diff
No declarations were harmed in the making of this PR! 🐙
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.
Decrease in tech debt: (relative, absolute) = (2.00, 0.00)
| Current number | Change | Type |
|---|---|---|
| 6751 | -2 | backward.isDefEq.respectTransparency |
Current commit 3a213a7faf
Reference commit 3d8100bbc6
You can run this locally as
./scripts/reporting/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).
|
!bench |
|
Benchmark results for 588a6c1 against 3d8100b are in. Significant changes detected! @JovanGerb
Large changes (10✅, 1🟥)
Medium changes (66✅, 14🟥) Too many entries to display here. View the full report on radar instead. Small changes (163✅, 69🟥) Too many entries to display here. View the full report on radar instead. |
|
!bench |
|
Benchmark results for 877ba14 against 3d8100b are in. Significant changes detected! @JovanGerb
Large changes (9✅, 1🟥)
Medium changes (65✅, 15🟥) Too many entries to display here. View the full report on radar instead. Small changes (154✅, 77🟥) Too many entries to display here. View the full report on radar instead. |
This reverts commit 877ba14.
|
!bench |
|
Benchmark results for 53446cc against 3d8100b are in. Significant changes detected! @JovanGerb
Large changes (10✅, 2🟥)
Medium changes (66✅, 15🟥) Too many entries to display here. View the full report on radar instead. Small changes (161✅, 67🟥) Too many entries to display here. View the full report on radar instead. |
|
!bench |
|
Benchmark results for 268a6a5 against 3d8100b are in. Significant changes detected! @JovanGerb
Large changes (10✅, 1🟥)
Medium changes (66✅, 15🟥) Too many entries to display here. View the full report on radar instead. Small changes (165✅, 67🟥) Too many entries to display here. View the full report on radar instead. |
grunweg
left a comment
There was a problem hiding this comment.
Thanks a lot for doing this; this looks great! I have a few clarification questions (and would maintainer merge otherwise/already merge some parts of it).
|
|
||
|
|
||
| noncomputable section | ||
| suppress_compilation -- needed to avoid a panic! |
There was a problem hiding this comment.
Can you minimize this? This seems worth reporting!
| end basic | ||
|
|
||
| variable [Fintype m] [NonUnitalCStarAlgebra A] [PartialOrder A] [StarOrderedRing A] | ||
| variable [Fintype m] [NonUnitalCStarAlgebra A] |
There was a problem hiding this comment.
Is there a reason for this change? (I like it regardless; if it compiles on its own, I'm happy to splice-bot it.)
There was a problem hiding this comment.
Yes, a linter suddenly told me that these hypotheses were unused.
There was a problem hiding this comment.
Split PR created
Split off the changes to Mathlib/Analysis/CStarAlgebra/CStarMatrix.lean in #37996.
| variable [Fintype ι] | ||
|
|
||
| lemma toLp_pi (p : ℝ≥0∞) [Fact (1 ≤ p)] (hX : HasGaussianLaw (fun ω ↦ (X · ω)) P) : | ||
| lemma toLp_pi [Finite ι] (p : ℝ≥0∞) [Fact (1 ≤ p)] (hX : HasGaussianLaw (fun ω ↦ (X · ω)) P) : |
There was a problem hiding this comment.
Thanks for the fix. I gather this is linter with the lower priority, but not now --- do you know why/ is there a deeper problem we should fix?
There was a problem hiding this comment.
Similarly to the other change, the linter now recognized that some type class assumption was unused. This is because the synthesis order changed, so a different instance was found. This does not necessarily mean that something is wrong, though it is a bit counter intuitive.
Co-authored-by: Michael Rothgang <rothgang@math.uni-bonn.de>
This is an attempt to improve performance by lowering the priority of some normed instances.
The idea is that when looking for algebraic instances, searching in the
Normedhierarchy is likely to be expensive and not useful.