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] - feat(topology/algebra/order): ⁻¹ continuous for linear ordered fields #15022

Closed
wants to merge 10 commits into from

Conversation

ericrbg
Copy link
Collaborator

@ericrbg ericrbg commented Jun 28, 2022

Closes #12781.


Open in Gitpod

@ericrbg ericrbg added awaiting-CI The author would like to see what CI has to say before doing more work. awaiting-review The author would like community review of the PR labels Jun 28, 2022
@ericrbg ericrbg requested a review from hrmacbeth June 28, 2022 01:25
@ericrbg
Copy link
Collaborator Author

ericrbg commented Jun 28, 2022

should we also make the topological_division_ring instance with this or is there TC loops with that? where would we put it?

@YaelDillies
Copy link
Collaborator

If this is also true for linear ordered semifields, then we will be able to get rid of nnreal.has_continuous_inv₀

@ericrbg
Copy link
Collaborator Author

ericrbg commented Jun 28, 2022

Oh, it should be from my memory. I'll try do that generalisation tomorrow.

@eric-wieser
Copy link
Member

We don't have a typeclass for linearly ordered semifields though, do we?

@ericrbg
Copy link
Collaborator Author

ericrbg commented Jun 28, 2022

No, we don't. And I just remembered I used a lot of subtraction (3am brain isn't great) - I think it can be worked around, but not 100% sure.

@eric-wieser
Copy link
Member

Maybe it's worth just leaving a TODO comment summarizing Yael's point (or asking @YaelDillies to suggest one themself); actually addressing it seems out of scope here.

@eric-wieser
Copy link
Member

should we also make the topological_division_ring instance with this or is there TC loops with that? where would we put it?

I think you'd be safe from typeclass loops, there aren't many instances of topological_division_ring.

@ericrbg
Copy link
Collaborator Author

ericrbg commented Jun 28, 2022

done; I'm curious how my structure building works there, I don't provide the topological_ring instance.

@YaelDillies
Copy link
Collaborator

YaelDillies commented Jun 28, 2022

#15027 for linear_ordered_semifield. Maybe we will also need topological_division_semiring? My point about nnreal.has_continuous_inv₀ is that in theory you should be able to copy-paste its proof.

@ericrbg
Copy link
Collaborator Author

ericrbg commented Jun 28, 2022

the proof seems to be based on the real coercion

@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 Jun 28, 2022
@ericrbg
Copy link
Collaborator Author

ericrbg commented Jul 5, 2022

Thanks, that works! How come I don't need to provide the topological_ring instance as well, and TC just picks it up?

@eric-wieser
Copy link
Member

eric-wieser commented Jul 5, 2022

I think that's because it's a new-style structure.

@ocfnash
Copy link
Collaborator

ocfnash commented Jul 11, 2022

I think it's worth adding a comment about generalising to the non-existent topological_division_semiring with application to nnreal but otherwise this LGTM.

bors d+

@bors
Copy link

bors bot commented Jul 11, 2022

✌️ ericrbg can now approve this pull request. To approve and merge a pull request, simply reply with bors r+. More detailed instructions are available here.

@github-actions github-actions bot added delegated The PR author may merge after reviewing final suggestions. and removed awaiting-review The author would like community review of the PR labels Jul 11, 2022
@ericrbg
Copy link
Collaborator Author

ericrbg commented Jul 13, 2022

bors r+

bors bot pushed a commit that referenced this pull request Jul 13, 2022
…#15022)

Closes #12781.



Co-authored-by: Eric Rodriguez <37984851+ericrbg@users.noreply.github.com>
@bors
Copy link

bors bot commented Jul 13, 2022

Pull request successfully merged into master.

Build succeeded:

@bors bors bot changed the title feat(topology/algebra/order): ⁻¹ continuous for linear ordered fields [Merged by Bors] - feat(topology/algebra/order): ⁻¹ continuous for linear ordered fields Jul 13, 2022
@bors bors bot closed this Jul 13, 2022
@bors bors bot deleted the ericrbg/continuous_inv branch July 13, 2022 21:11
joelriou pushed a commit that referenced this pull request Jul 23, 2022
…#15022)

Closes #12781.



Co-authored-by: Eric Rodriguez <37984851+ericrbg@users.noreply.github.com>
bors bot pushed a commit that referenced this pull request Nov 15, 2022
…17549)

File authorship credits go to:
* Benjamin Davidson: #4138
* Devon Tuma: #7545
* Eric Rodriguez: #15022
* Oliver Nash: #16762
bors bot pushed a commit that referenced this pull request Nov 16, 2022
…17549)

File authorship credits go to:
* Benjamin Davidson: #4138
* Devon Tuma: #7545
* Eric Rodriguez: #15022
* Oliver Nash: #16762
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
delegated The PR author may merge after reviewing final suggestions.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

has_continuous_inv₀ instance for linear_ordered_field
4 participants