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

feat(data/rat): redefine rat.lt so that apply works #18

Closed

Conversation

robertylewis
Copy link
Member

I noticed the following a little while ago, and worked around it, but it came up again.

import data.rat

theorem f2 (T : Type) (h : ordered_comm_group T) : (0 : T) < 0 := sorry

example : (0 : ℤ) < 0 :=
begin 
apply f2
end

example : (0 : ℚ) < 0 :=
begin 
apply f2 -- fails
end

When apply chooses how many arguments to instantiate with metavariables, it does some unfolding. When lt is defined as the negation of le, the count is off by one and apply fails. There are other possible fixes, I think, but this one works for now.

This is based on 3.3.0 since I'm working with that version right now, but I think the same patch should apply to master; I don't remember how we decided to handle these kinds of fixes.

@digama0
Copy link
Member

digama0 commented Oct 25, 2017

The latest commit resolves this issue by using the default implementation of lt. The reason it was implemented that way in the first place was so that the decidable instance wouldn't have to calculate two le relations, but that can be done (and is now done) in the decidable instance itself.

@digama0 digama0 closed this Oct 25, 2017
@robertylewis
Copy link
Member Author

@digama0 Thanks. Would it be possible to apply this fix (or your version of this fix, it doesn't matter) to the v3.3.0 branch? I want to keep this project on a stable version.

digama0 added a commit that referenced this pull request Oct 25, 2017
@digama0
Copy link
Member

digama0 commented Oct 25, 2017

Backported

gebner pushed a commit to gebner/mathlib that referenced this pull request Apr 3, 2020
Testing to see if bors can merge this
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants