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(data/real/basic): make and computable on reals #16463

Closed
wants to merge 34 commits into from

Conversation

eric-wieser
Copy link
Member

@eric-wieser eric-wieser commented Sep 11, 2022

This also makes norm and dist computable on real, nnreal, rat, and int.

From a mathematical point of view that doesn't care about computability, this still contains the (mildly) interesting result that the maximum of two reals is equal to the real produced by taking the elementwise maximum of the two cauchy sequences.

This change causes some unpleasant elaboration pain in modular.lean for reasons presumably linked to unification of dependent typeclasses. It's not clear to me precisely what the cause if.


Open in Gitpod

If the unification issues are particular cause for concern, then I could try to bisect to find which change is responsible.

@eric-wieser eric-wieser added WIP Work in progress awaiting-CI The author would like to see what CI has to say before doing more work. labels Sep 11, 2022
@mathlib-dependent-issues-bot mathlib-dependent-issues-bot added the blocked-by-other-PR This PR depends on another PR which is still in the queue. A bot manages this label via PR comment. label Sep 12, 2022
@github-actions github-actions bot removed the awaiting-CI The author would like to see what CI has to say before doing more work. label Sep 14, 2022
@eric-wieser eric-wieser removed the WIP Work in progress label Sep 15, 2022
@YaelDillies
Copy link
Collaborator

Why is prod.seminormed_add_comm_group still noncomputable? max is just sup again.

@eric-wieser
Copy link
Member Author

Why is prod.seminormed_add_comm_group still noncomputable? max is just sup again.

Because I didn't want to make this PR grow any further, and had intended to leave that to a followup.
The weird timeout already has me uncomfortable with the size of this, which is why I've been trying to shrink the PR a little.

My guess would be that it's because min is noncomputable but inf is computable.

@YaelDillies
Copy link
Collaborator

That feels like good enough justification to rename max/min to sup/inf (and possibly leave max/min around as notation). I will PR this to core.

@eric-wieser
Copy link
Member Author

Another option would be to keep min/max but for is_total_order (\le) or similar.

@eric-wieser eric-wieser added the awaiting-review The author would like community review of the PR label Sep 19, 2022
@mathlib-dependent-issues-bot mathlib-dependent-issues-bot removed the blocked-by-other-PR This PR depends on another PR which is still in the queue. A bot manages this label via PR comment. label Sep 19, 2022
@eric-wieser eric-wieser added the t-analysis Analysis (normed *, calculus) label Sep 19, 2022
Copy link
Member

@jcommelin jcommelin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks 🎉

bors merge

@leanprover-community-bot-assistant leanprover-community-bot-assistant 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 Sep 20, 2022
bors bot pushed a commit that referenced this pull request Sep 20, 2022
This also makes `norm` and `dist` computable on `real`, `nnreal`, `rat`, and `int`.

From a mathematical point of view that doesn't care about computability, this still contains the (mildly) interesting result that the maximum of two reals is equal to the real produced by taking the elementwise maximum of the two cauchy sequences.

This change causes some unpleasant elaboration pain in `modular.lean` for reasons presumably linked to unification of dependent typeclasses. It's not clear to me precisely what the cause if.



Co-authored-by: Yaël Dillies <yael.dillies@gmail.com>
@bors
Copy link

bors bot commented Sep 20, 2022

Pull request successfully merged into master.

Build succeeded:

@bors bors bot changed the title refactor(data/real/basic): make and computable on reals [Merged by Bors] - refactor(data/real/basic): make and computable on reals Sep 20, 2022
@bors bors bot closed this Sep 20, 2022
@bors bors bot deleted the eric-wieser/real-computable-min branch September 20, 2022 08:56
b-mehta pushed a commit that referenced this pull request Sep 21, 2022
This also makes `norm` and `dist` computable on `real`, `nnreal`, `rat`, and `int`.

From a mathematical point of view that doesn't care about computability, this still contains the (mildly) interesting result that the maximum of two reals is equal to the real produced by taking the elementwise maximum of the two cauchy sequences.

This change causes some unpleasant elaboration pain in `modular.lean` for reasons presumably linked to unification of dependent typeclasses. It's not clear to me precisely what the cause if.



Co-authored-by: Yaël Dillies <yael.dillies@gmail.com>
bors bot pushed a commit that referenced this pull request Sep 27, 2022
… instance (#16659)

This just removes a shortcut instance in `number_theory/legendre_symnol/norm_num` that was necessary to avoid computability trouble with the meta code, but is no longer so, since the root cause was fixed by #16463.
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

5 participants