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.

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] - refactor(linear_algebra): morphism refactor for linear_equiv, continuous_linear_equiv, linear_isometry and linear_isometry_equiv #15078

Closed
wants to merge 9 commits into from

Conversation

dupuisf
Copy link
Collaborator

@dupuisf dupuisf commented Jun 30, 2022

This PR brings the morphism refactor to linear_equiv, continuous_linear_equiv, linear_isometry, and linear_isometry_equiv.

Note that I had to resort to an ugly hack to deal with deterministic timouts in two proofs: vitali.exists_disjoint_covering_ae and another one in the counterexamples folder. Both of these are very large proofs with no obvious sublemmas to extract or inefficiencies, so I just used try_for 20000 {...} to increase the timeout. It's not great, but I don't see a better alternative -- besides, I don't think it's unreasonable for long proofs like these to have a bigger timeout.


Open in Gitpod

@dupuisf dupuisf added the awaiting-review The author would like community review of the PR label Jun 30, 2022
@Vierkantor Vierkantor requested a review from sgouezel July 12, 2022 13:39
@Vierkantor
Copy link
Collaborator

The morphism refactor part looks good, but I'd like to see a bit more work being done to address the timeout. @sgouezel, since you appear in the blame of both proofs that are timing out, do you have any suggestions?

@dupuisf dupuisf added the t-analysis Analysis (normed *, calculus) label Jul 29, 2022
@dupuisf
Copy link
Collaborator Author

dupuisf commented Jul 29, 2022

I did some additional digging into the timeout problem in counterexamples/philips.lean, and type class search goes wild in several places in that proof in search of has_coe_to_fun ↥A ?x_4. I have no idea why it looks for that instance, but it does so several times and has to go through all the fun_like instances in the import tree as far as I can tell.

I don't have time to debug this myself before going on vacation, so I just thought I would record this here for now.

@sgouezel
Copy link
Collaborator

sgouezel commented Aug 2, 2022

This has been discussed in Zulip, in https://leanprover.zulipchat.com/#narrow/stream/113488-general/topic/has_coe_to_fun.20instance.20search/near/291439013. The issue is that Lean tries to find a coercion to function spaces when the target is a pi type, including set α. I have added to the problematic proofs a bunch of coercion annotations, writing ↑s instead of s to help the system. Let's see if this fixes the timeouts.

@sgouezel
Copy link
Collaborator

sgouezel commented Aug 3, 2022

bors r+
Thanks!

@github-actions github-actions bot added ready-to-merge All that is left is for bors to build and merge this PR. (Remember you need to say `bors r+`.) and removed awaiting-review The author would like community review of the PR labels Aug 3, 2022
bors bot pushed a commit that referenced this pull request Aug 3, 2022
…inuous_linear_equiv`, `linear_isometry` and `linear_isometry_equiv` (#15078)

This PR brings the morphism refactor to `linear_equiv`, `continuous_linear_equiv`, `linear_isometry`, and `linear_isometry_equiv`.

Note that I had to resort to an ugly hack to deal with deterministic timouts in two proofs: `vitali.exists_disjoint_covering_ae` and another one in the counterexamples folder. Both of these are very large proofs with no obvious sublemmas to extract or inefficiencies, so I just used `try_for 20000 {...}` to increase the timeout. It's not great, but I don't see a better alternative -- besides, I don't think it's unreasonable for long proofs like these to have a bigger timeout.



Co-authored-by: sgouezel <sebastien.gouezel@univ-rennes1.fr>
@bors
Copy link

bors bot commented Aug 3, 2022

Build failed (retrying...):

bors bot pushed a commit that referenced this pull request Aug 3, 2022
…inuous_linear_equiv`, `linear_isometry` and `linear_isometry_equiv` (#15078)

This PR brings the morphism refactor to `linear_equiv`, `continuous_linear_equiv`, `linear_isometry`, and `linear_isometry_equiv`.

Note that I had to resort to an ugly hack to deal with deterministic timouts in two proofs: `vitali.exists_disjoint_covering_ae` and another one in the counterexamples folder. Both of these are very large proofs with no obvious sublemmas to extract or inefficiencies, so I just used `try_for 20000 {...}` to increase the timeout. It's not great, but I don't see a better alternative -- besides, I don't think it's unreasonable for long proofs like these to have a bigger timeout.



Co-authored-by: sgouezel <sebastien.gouezel@univ-rennes1.fr>
@bors
Copy link

bors bot commented Aug 3, 2022

Pull request successfully merged into master.

Build succeeded:

@bors bors bot changed the title refactor(linear_algebra): morphism refactor for linear_equiv, continuous_linear_equiv, linear_isometry and linear_isometry_equiv [Merged by Bors] - refactor(linear_algebra): morphism refactor for linear_equiv, continuous_linear_equiv, linear_isometry and linear_isometry_equiv Aug 3, 2022
@bors bors bot closed this Aug 3, 2022
@bors bors bot deleted the dupuisf/linear_equiv_class branch August 3, 2022 14:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ready-to-merge All that is left is for bors to build and merge this PR. (Remember you need to say `bors r+`.) t-analysis Analysis (normed *, calculus)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants