perf(LinearAlgebra) remove a simp for Module.finrank#38401
perf(LinearAlgebra) remove a simp for Module.finrank#38401mcdoll wants to merge 2 commits intoleanprover-community:masterfrom
simp for Module.finrank#38401Conversation
PR summary ee3a5404e5Import changes for modified filesNo significant changes to the import graph Import changes for all files
Declarations diffNo 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 No changes to technical debt.This script lives in the
|
| rw [← not_le] at this | ||
| simp [Module.finrank, this, Module.rank_zero_iff_of_free] | ||
|
|
||
| @[simp] |
There was a problem hiding this comment.
Maybe we can still tag this nontriviality, assuming the attribute is still around
There was a problem hiding this comment.
If it is the case that this lemma was used exactly once, then I don't know whether we should tag it anything.
There was a problem hiding this comment.
Shouldn't the question be "can this be a useful nontriviality lemma in the future" rather than "is it right now"? (I don't in this area a lot, so you might be a better judge of that.)
|
!radar |
|
Benchmark results for dfdcfd6 against ee3a540 are in. No significant results found. @mcdoll
Medium changes (3✅)
Small changes (10✅)
|
|
Seems to be a reasonably good improvement |
This is a bad
simplemma because it gets tried on any termModule.finrank _ _and then invokes the typeclass inference to check the assumptions. Both of the assumptions are rarely satisfied, but typeclass inference might take a while to figure that out, hence causing a slowdown for everysimpcall that containsModule.finrank _ _.