Skip to content
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.

Sign up for GitHub

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

Jump to bottom

joachim/implicitDefEqProofs docstring #5750

Closed
wants to merge 2 commits into from
Closed

joachim/implicitDefEqProofs docstring #5750

wants to merge 2 commits into from

Conversation

nomeata
Copy link
Contributor

@nomeata nomeata commented Oct 17, 2024

  • feat: DiscrTree: index the domain of ∀
  • docs: remove docstring from implicitDefEqProofs

@nomeata
feat: DiscrTree: index the domain of ∀
313e313 
It bothered me that inferring instances of the shape `Decidable (∀ (x : Fin _), _)`
will go linearly through all instances of that shape, even those that are
about `∀ (x : Nat), …`. And that  `Decidable (∃ (x : Fin _), _)` gets better
indexing than `Decidable (∀ (x : Fin _), _)`.

Judging from code comments, the discr tree used to index arrow types
with two arguments (domain and body), and that led to bugs due to the
dependency, so the arguments were removed. But it seems that indexing
the domain is completely simple and innocent.

So let’s see what happens…

Mostly only insignificant perf improvements, unfortunately (~Mathlib.Data.Matroid.IndepAxioms — instructions -11.4B, overall build instructions -0.097 %):
http://speed.lean-fro.org/mathlib4/compare/dd333cc1-fa26-42f2-96c6-b0e66047d0b6/to/6875ff8f-a17c-431d-8b8b-2f00799be794

This is just a small baby step compared to the more invasive improvements
done in the [`RefinedDiscrTree` by  J. W. Gerbscheid](https://leanprover-community.github.io/mathlib4_docs/Mathlib/Tactic/FunProp/RefinedDiscrTree.html) in mathlib.
@nomeata
docs: remove docstring from implicitDefEqProofs
d24d4c1 
this option was added in fb97275 to
prepare for #4595, due to boostrapping issues, but #4595 has not landed
yet. This is be very confusing when people discover this option and try
to use it (as I did).

So let's clearly mark this as not yet implemented on `main`.
@nomeata nomeata closed this Oct 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
1 participant
@nomeata