-
Notifications
You must be signed in to change notification settings - Fork 297
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/nat/factorization): Change definition of factorization
to be computable
#12301
Conversation
I think if you want to do this, the right way is to make a computable |
As Reid said here, the algorithm to compute nat.factors is one you would never want to use. In particular, I think that if we do this by making a computable |
… BoltonBailey/computable-factorization
The proofs of |
Thanks for the helpful comments, I'll wait to implement them until this refactor of |
… BoltonBailey/computable-factorization
Your most recent commit seems to be deleting the entire file |
Yes, I spun off a change to that file into a new PR, and then I hit delete, thinking it would revert my changes. Sort of my fault because I guess it did what it said it was going to do, but why there's a delete file option but not a revert file option seems strange to me. I'll add it back when I merge #13588 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks!
bors d+
✌️ BoltonBailey can now approve this pull request. To approve and merge a pull request, simply reply with |
Co-authored-by: Anne Baanen <Vierkantor@users.noreply.github.com>
Co-authored-by: Anne Baanen <Vierkantor@users.noreply.github.com>
Co-authored-by: Anne Baanen <Vierkantor@users.noreply.github.com>
Co-authored-by: Anne Baanen <Vierkantor@users.noreply.github.com>
Co-authored-by: Anne Baanen <Vierkantor@users.noreply.github.com>
Co-authored-by: Anne Baanen <Vierkantor@users.noreply.github.com>
Co-authored-by: Anne Baanen <Vierkantor@users.noreply.github.com>
bors r+ |
… BoltonBailey/computable-factorization
Canceled. |
bors r- |
…b.com/leanprover-community/mathlib into BoltonBailey/computable-factorization
Ok, trying again |
bors r+ |
…` to be computable (#12301) This PR changes the definition of `data.nat.factorization` to not depend on `multiset.to_finsupp` and instead to depend on `padic_val_nat`. This sidesteps the computability issues with finsupp discussed in [this thread](https://leanprover.zulipchat.com/#narrow/stream/113489-new-members/topic/.60ite.60.20with.20multiple.20decidable.20instances/near/272409877). To deal with the changed imports this PR also moves some material from `number_theory/padics/padic_val` and `ring_theory/multiplicity` into `data/nat/factorization/basic`. Co-authored-by: Stuart Presnell <stuart.presnell@gmail.com> Co-authored-by: Stuart Presnell <stuart.presnell@gmail.com>
Pull request successfully merged into master. Build succeeded: |
factorization
to be computablefactorization
to be computable
…` to be computable (#12301) This PR changes the definition of `data.nat.factorization` to not depend on `multiset.to_finsupp` and instead to depend on `padic_val_nat`. This sidesteps the computability issues with finsupp discussed in [this thread](https://leanprover.zulipchat.com/#narrow/stream/113489-new-members/topic/.60ite.60.20with.20multiple.20decidable.20instances/near/272409877). To deal with the changed imports this PR also moves some material from `number_theory/padics/padic_val` and `ring_theory/multiplicity` into `data/nat/factorization/basic`. Co-authored-by: Stuart Presnell <stuart.presnell@gmail.com> Co-authored-by: Stuart Presnell <stuart.presnell@gmail.com>
This PR changes the definition of
data.nat.factorization
to not depend onmultiset.to_finsupp
and instead to depend onpadic_val_nat
. This sidesteps the computability issues with finsupp discussed in this thread.To deal with the changed imports this PR also moves some material from
number_theory/padics/padic_val
andring_theory/multiplicity
intodata/nat/factorization/basic
.Co-authored-by: Stuart Presnell stuart.presnell@gmail.com