Skip to content

[Merged by Bors] - fix: out-line instance equality proofs in norm_num#4048

Closed
digama0 wants to merge 5 commits intomasterfrom
instance_norm_num
Closed

[Merged by Bors] - fix: out-line instance equality proofs in norm_num#4048
digama0 wants to merge 5 commits intomasterfrom
instance_norm_num

Conversation

@digama0
Copy link
Copy Markdown
Member

@digama0 digama0 commented May 17, 2023

This applies a similar technique as was used for the ofScientific norm_num extension to the core field operations. This ensures that lean doesn't try to prove Add.add (inst := inst1) 1234 56 = Add.add (inst := inst2) 1234 56 by evaluating the addition itself, and instead tries to prove the instances equal and let norm_num do the adding. Reported at #4036 (comment) , although I can't find a test case that doesn't involve the base norm_num implementation because all the core operations on nat are already overridden.


Open in Gitpod

@digama0 digama0 added awaiting-review t-meta Tactics, attributes or user commands labels May 19, 2023
@fpvandoorn
Copy link
Copy Markdown
Member

  • Do we have to mimic this change in the other norm num extensions to get the correct behavior?
  • Can you add some of the tests now that some norm num extensions are merged, so you can test whether this works now?
  • Can you add a bit of doc (or a comment) why we need to formulate the isNat lemmas using an equality between the functions (repeating part of the commit message, but in a place that is visible after the PR is merged)?

@fpvandoorn fpvandoorn added awaiting-author A reviewer has asked the author a question or requested changes. and removed awaiting-review labels May 23, 2023
@digama0
Copy link
Copy Markdown
Member Author

digama0 commented May 23, 2023

Do we have to mimic this change in the other norm num extensions to get the correct behavior?

Only the ones that involve a typeclass search that could be resolved in several ways. So for example the Nat.div extension probably doesn't require this, nor most of the named functions. + - * are special since they can come from several sources and there are some complicated typeclass problems that can be kicked up.

@digama0 digama0 added awaiting-review and removed awaiting-author A reviewer has asked the author a question or requested changes. labels May 27, 2023
@fpvandoorn
Copy link
Copy Markdown
Member

Thanks!

bors merge

@github-actions github-actions bot added ready-to-merge This PR has been sent to bors. and removed awaiting-review labels May 28, 2023
bors bot pushed a commit that referenced this pull request May 28, 2023
This applies a similar technique as was used for the `ofScientific` norm_num extension to the core field operations. This ensures that lean doesn't try to prove `Add.add (inst := inst1) 1234 56 = Add.add (inst := inst2) 1234 56` by evaluating the addition itself, and instead tries to prove the instances equal and let norm_num do the adding. Reported at #4036 (comment) , although I can't find a test case that doesn't involve the base norm_num implementation because all the core operations on nat are already overridden.
@bors
Copy link
Copy Markdown

bors bot commented May 28, 2023

Pull request successfully merged into master.

Build succeeded!

The publicly hosted instance of bors-ng is deprecated and will go away soon.

If you want to self-host your own instance, instructions are here.
For more help, visit the forum.

If you want to switch to GitHub's built-in merge queue, visit their help page.

@bors bors bot changed the title fix: out-line instance equality proofs in norm_num [Merged by Bors] - fix: out-line instance equality proofs in norm_num May 28, 2023
@bors bors bot closed this May 28, 2023
@bors bors bot deleted the instance_norm_num branch May 28, 2023 20:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ready-to-merge This PR has been sent to bors. t-meta Tactics, attributes or user commands

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants