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(order/filter/filter_product): build hyperreals #801

Merged
merged 6 commits into from Mar 17, 2019
Merged

Conversation

abhimanyupallavisudhir
Copy link
Collaborator

Construction of "filter products" (ultraproducts on a general filter), ultraproducts, some instances, hyperreal numbers. Not sure if it belongs in another folder.

@sgouezel
Copy link
Collaborator

sgouezel commented Mar 8, 2019

Comments on the form:

  • You should add a header, with the license details and so on, but also explaining what is in the file, with motivations, math explanations, and references to the literature.
  • Add docstrings to most of the definitions
  • You should use namespaces: put the first definitions in the filter namespace, and the instances maybe in filter.bigly_equal.

Otherwise, the maths look good (except that you are missing a field instance when your filter is a ultrafilter).

src/order/filter/filter_product.lean Show resolved Hide resolved
src/order/filter/filter_product.lean Outdated Show resolved Hide resolved
src/order/filter/filter_product.lean Outdated Show resolved Hide resolved
src/order/filter/filter_product.lean Outdated Show resolved Hide resolved
src/order/filter/filter_product.lean Outdated Show resolved Hide resolved
src/order/filter/filter_product.lean Outdated Show resolved Hide resolved
src/order/filter/filter_product.lean Outdated Show resolved Hide resolved
src/order/filter/basic.lean Outdated Show resolved Hide resolved
src/order/filter/basic.lean Outdated Show resolved Hide resolved
src/order/filter/basic.lean Outdated Show resolved Hide resolved
Copy link
Collaborator

@johoelzl johoelzl left a comment

Choose a reason for hiding this comment

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

Hi @abhimanyupallavisudhir,
the formalization looks really nice. I have a couple of style comments and requests for changes.

src/order/filter/basic.lean Outdated Show resolved Hide resolved
src/order/filter/basic.lean Outdated Show resolved Hide resolved
src/order/filter/basic.lean Outdated Show resolved Hide resolved
src/order/filter/filter_product.lean Outdated Show resolved Hide resolved
src/order/filter/filter_product.lean Outdated Show resolved Hide resolved
src/order/filter/filter_product.lean Outdated Show resolved Hide resolved
src/order/filter/filter_product.lean Outdated Show resolved Hide resolved
src/order/filter/filter_product.lean Outdated Show resolved Hide resolved
src/order/filter/filter_product.lean Outdated Show resolved Hide resolved
src/order/filter/filter_product.lean Outdated Show resolved Hide resolved
Construction of filter products, ultraproducts, some instances, hyperreal numbers.
@avigad
Copy link
Collaborator

avigad commented Mar 16, 2019

@abhimanyupallavisudhir I already made the fix and checked compilation on my machine. I am just waiting for the Travis check to finish, and then I'll merge.

@abhimanyupallavisudhir
Copy link
Collaborator Author

@abhimanyupallavisudhir I already made the fix and checked compilation on my machine. I am just waiting for the Travis check to finish, and then I'll merge.

Isn't making f implicit a better fix?

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.

I realise I'm a bit late with these comments, as @avigad was already talking about merging this. I don't mind if you ignore these comments... they're not that important.

I should stress that I think this is a cool PR!

src/data/real/hyperreal.lean Outdated Show resolved Hide resolved
src/order/filter/filter_product.lean Outdated Show resolved Hide resolved
@avigad
Copy link
Collaborator

avigad commented Mar 16, 2019

@abhimanyupallavisudhir I didn't even see your change. I think we pushed them at the same time. Could you make the argument implicit again and adapt @jcommelin's suggestions as you see fit? Then I'll merge right away.

@abhimanyupallavisudhir
Copy link
Collaborator Author

Ok, all done.

@avigad avigad merged commit e8bdc7f into master Mar 17, 2019
@johoelzl johoelzl deleted the filter_product branch March 19, 2019 10:11
anrddh pushed a commit to anrddh/mathlib that referenced this pull request May 15, 2020
…nity#801)

Construction of filter products, ultraproducts, some instances, hyperreal numbers.
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

6 participants