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

attribute [class] nat.prime in data.padics.padic_norm has awkward side effects #1601

Closed
rwbarton opened this issue Oct 23, 2019 · 6 comments
Closed

Comments

@rwbarton
Copy link
Collaborator

A side effect of making nat.prime into a class is that you can no longer revert a nat.prime p hypothesis that came from the left of the colon, or (more likely) revert p itself, in order to subst it, for example.

data.padics.padic_norm has attribute [class] nat.prime, so that the primality of p can be taken as a type class argument to most of the functions in the file. But this means that if, for example, you import data.padics.padic_norm in data.zmod.quadratic_reciprocity, then some of the proofs there will break because they rely on substing parameters that are known to be prime. Now adding that import is not that likely, but it could happen (#1564) that some random module that data.zmod.quadratic_reciprocity imports gains an import of something else that imports data.padics.padic_norm. It's probably possible to refactor things in this case to avoid the problematic transitive import, but that just pushes the issue down the road.

Two obvious possible solutions:

  • Use local attribute [class] nat.prime in data.padics.padic_norm, and make its users do the same.
  • "Un-orphan" attribute [class] nat.prime by moving it to the definition of nat.prime, and fix all the proofs that break as a result.
@jcommelin
Copy link
Member

There is another option. I haven't thought it through yet, but I'd love to see it smashed to pieces.

Crazy idea: We throw away the attribute [class] nat.prime in favor of nat.primes that was introduced some time ago. Z_p and Q_p take a term of type nat.primes as argument. We remove the type wrapper zmodp and put all its instance on top of zmod p, after introducing a coercion from nat.primes to pnat.

@robertylewis
Copy link
Member

Just realized I never commented on this, and should, because I'm guilty of introducing attribute [class] nat.prime. I think, in order, my preferred solutions are

  1. "Un-orphan" attribute [class] nat.prime by moving it to the definition of nat.prime. But I don't know how much this will break and how ugly the fixes will be. It will also sometimes lead to error messages about unfreezing hypotheses that will be confusing out of this context.
  2. We throw away the attribute [class] nat.prime in favor of nat.primes. This will have the effect of inserting (even more) casts throughout the padics development. This will be less of a problem than when I did the padics because norm_cast exists now, but it won't be trivial.
  3. Use local attribute [class] nat.prime in data.padics.padic_norm. This should be implemented as a locale, and if we can avoid it, I'd prefer not to require users to open a locale in order to use a theory. It's probably the least destructive option though.

@jcommelin
Copy link
Member

I think I agree with your suggested order. If nobody beats me to it, I might start the refactor in two weeks time.

@jcommelin
Copy link
Member

#2511 is my attempt to address this issue. It doesn't touch the padics yet, but that would be the immediate next step.

bors bot pushed a commit that referenced this issue Apr 24, 2020
…c search (#2511)

This PR introduces the class `fact P := P` for `P : Prop`, which is a way to inject elementary propositions into the type class system. This desing is inspired by @cipher1024 's https://gist.github.com/cipher1024/9bd785a75384a4870b331714ec86ad6f#file-zmod_redesign-lean.

We use this to endow `zmod p` with a `field` instance under the assumption `[fact p.prime]`.
As a consequence, the type `zmodp` is no longer needed, which removes a lot of duplicate code.
Besides that, we redefine `zmod n`, so that `n` is a natural number instead of a *positive* natural number, and `zmod 0` is now definitionally the integers.
To preserve computational properties, `zmod n` is not defined as quotient type, but rather as `int` and `fin n`, depending on whether `n` is `0` or `(k+1)`.

The rest of this PR is adapting the library to the new definitions (most notably quadratic reciprocity and Lagrange's four squares theorem).

Future work: Refactor the padics to use `[fact p.prime]` instead of making `nat.prime` a class in those files. This will address #1601 and #1648. Once that is done, we can clean up the mess in `char_p` (where the imports are really tangled) and finally get some movement in #1564.
@jcommelin
Copy link
Member

The next steps are #2518 and #2519

@jcommelin
Copy link
Member

#2519 has now been merged, so I consider this issue fixed.

anrddh pushed a commit to anrddh/mathlib that referenced this issue May 15, 2020
…c search (leanprover-community#2511)

This PR introduces the class `fact P := P` for `P : Prop`, which is a way to inject elementary propositions into the type class system. This desing is inspired by @cipher1024 's https://gist.github.com/cipher1024/9bd785a75384a4870b331714ec86ad6f#file-zmod_redesign-lean.

We use this to endow `zmod p` with a `field` instance under the assumption `[fact p.prime]`.
As a consequence, the type `zmodp` is no longer needed, which removes a lot of duplicate code.
Besides that, we redefine `zmod n`, so that `n` is a natural number instead of a *positive* natural number, and `zmod 0` is now definitionally the integers.
To preserve computational properties, `zmod n` is not defined as quotient type, but rather as `int` and `fin n`, depending on whether `n` is `0` or `(k+1)`.

The rest of this PR is adapting the library to the new definitions (most notably quadratic reciprocity and Lagrange's four squares theorem).

Future work: Refactor the padics to use `[fact p.prime]` instead of making `nat.prime` a class in those files. This will address leanprover-community#1601 and leanprover-community#1648. Once that is done, we can clean up the mess in `char_p` (where the imports are really tangled) and finally get some movement in leanprover-community#1564.
anrddh pushed a commit to anrddh/mathlib that referenced this issue May 16, 2020
…c search (leanprover-community#2511)

This PR introduces the class `fact P := P` for `P : Prop`, which is a way to inject elementary propositions into the type class system. This desing is inspired by @cipher1024 's https://gist.github.com/cipher1024/9bd785a75384a4870b331714ec86ad6f#file-zmod_redesign-lean.

We use this to endow `zmod p` with a `field` instance under the assumption `[fact p.prime]`.
As a consequence, the type `zmodp` is no longer needed, which removes a lot of duplicate code.
Besides that, we redefine `zmod n`, so that `n` is a natural number instead of a *positive* natural number, and `zmod 0` is now definitionally the integers.
To preserve computational properties, `zmod n` is not defined as quotient type, but rather as `int` and `fin n`, depending on whether `n` is `0` or `(k+1)`.

The rest of this PR is adapting the library to the new definitions (most notably quadratic reciprocity and Lagrange's four squares theorem).

Future work: Refactor the padics to use `[fact p.prime]` instead of making `nat.prime` a class in those files. This will address leanprover-community#1601 and leanprover-community#1648. Once that is done, we can clean up the mess in `char_p` (where the imports are really tangled) and finally get some movement in leanprover-community#1564.
cipher1024 pushed a commit to cipher1024/mathlib that referenced this issue Mar 15, 2022
…c search (leanprover-community#2511)

This PR introduces the class `fact P := P` for `P : Prop`, which is a way to inject elementary propositions into the type class system. This desing is inspired by @cipher1024 's https://gist.github.com/cipher1024/9bd785a75384a4870b331714ec86ad6f#file-zmod_redesign-lean.

We use this to endow `zmod p` with a `field` instance under the assumption `[fact p.prime]`.
As a consequence, the type `zmodp` is no longer needed, which removes a lot of duplicate code.
Besides that, we redefine `zmod n`, so that `n` is a natural number instead of a *positive* natural number, and `zmod 0` is now definitionally the integers.
To preserve computational properties, `zmod n` is not defined as quotient type, but rather as `int` and `fin n`, depending on whether `n` is `0` or `(k+1)`.

The rest of this PR is adapting the library to the new definitions (most notably quadratic reciprocity and Lagrange's four squares theorem).

Future work: Refactor the padics to use `[fact p.prime]` instead of making `nat.prime` a class in those files. This will address leanprover-community#1601 and leanprover-community#1648. Once that is done, we can clean up the mess in `char_p` (where the imports are really tangled) and finally get some movement in leanprover-community#1564.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants