refactor(Data/Nat/Factorization/Defs): redefine Nat.factorization in terms of primeFactorsList instead of padicValNat - #43581
Conversation
PR summary c52578998dImport changes for modified filesNo significant changes to the import graph Import changes for all files
|
| of `p` in the factorization of `n`: we declare the former to be the simp-normal form. -/ | ||
| @[simp] | ||
| theorem primeFactorsList_count_eq {n p : ℕ} : n.primeFactorsList.count p = n.factorization p := by | ||
| theorem factorization_def (n : ℕ) {p : ℕ} (pp : p.Prime) : n.factorization p = padicValNat p n := by |
There was a problem hiding this comment.
Do you want to now rename this, and deprecate this _def version?
| theorem factorization_def (n : ℕ) {p : ℕ} (pp : p.Prime) : n.factorization p = padicValNat p n := by | |
| theorem factorization_eq_padicValNat (n : ℕ) {p : ℕ} (pp : p.Prime) : n.factorization p = padicValNat p n := by |
There was a problem hiding this comment.
Well, we want both statements (connecting Nat.factorization with both Nat.primeFactorsList and padicValNat (which will eventually be replaced by multiplicity)), and I thought that renaming might be a bit confusing deprecation-wise for now.
Hold on, can't we define Nat.factorization in terms of Nat.maxPowDvdDiv, which is the efficient one used by padicValNat anyway? Then we get computability, efficiency, and we can safely kill |
|
If you don't like my previous idea, then maybe Multiset.toFinsupp should be used, since you're building the finsupp out of a list (via a multiset) anyway |
|
|
It's not any more efficient unfortunately, since you still need to compute |
that's so annoying - it's just because it's bundled. the .toFun field is computable :/ |
|
Yeah, maybe it should be split to |
This PR redefines
Nat.factorizationin terms ofprimeFactorsListinstead ofpadicValNat. This will allowNat.factorizationto remain computable even aspadicValNatis deprecated in favor of the non-computablemultiplicity.