proofs for parts of RationalFunctions.lean#304
proofs for parts of RationalFunctions.lean#304alexanderlhicks wants to merge 2 commits intoVerified-zkEVM:mainfrom
Conversation
🤖 Gemini PR SummaryCompletes the formalization of Appendix A from the BCIKS20 paper within Mathematical Formalization
Codebase Impact
Analysis of Changes
Lean Declarations ✏️ **Removed:** 7 declaration(s)
✏️ **Added:** 13 declaration(s)
✏️ **Affected:** 5 declaration(s) (line number changed)
✅ **Removed:** 3 `sorry`(s)
❌ **Added:** 1 `sorry`(s)
✏️ **Affected:** 1 `sorry`(s) (line number changed)
🎨 **Style Guide Adherence**Based on the ArkLib style guide, here are the violations in the provided diff:
📄 **Per-File Summaries**
Last updated: 2026-03-04 18:54 UTC. |
katyhr
left a comment
There was a problem hiding this comment.
@alexanderlhicks I reviewed this and left some comments :)
We probably want to move the appendix A stuff somewhere else I would have thought? It's not general RatFunc stuff, it's all specific to the BCIKS paper
| ] | ||
| exact irreducibleHTildeOfIrreducible hdeg h | ||
|
|
||
| /-- The function field `𝕃` as defined above is a field. -/ |
There was a problem hiding this comment.
again do we want to keep these Facts? Irreducibility and non-zero natDegree are not typeclasses, right, they're props so if we want to do it in mathlib style we should do
noncomputable instance {H : F[X][Y]} (hirr : Irreducible H) (hdeg : H.natDegree ≠ 0) : Field (𝕃 H) := IsField.toField (isField_of_irreducible (H := H) hdeg hirr)
There was a problem hiding this comment.
This might break some stuff in the next section though. Happy to try to fix if we decide to go with the hypothesis vs Fact approach so we're closer to mathlib style
There was a problem hiding this comment.
mathlib also uses fact e.g. https://leanprover-community.github.io/mathlib4_docs/Mathlib/RingTheory/AdjoinRoot.html#AdjoinRoot.span_maximal_of_irreducible (pulled this example randomly). No strong opinion on my end really, but using a hypothesis would break typeclass resolution I think (or you'd add an analogous step in the proof to resolve this).
| noncomputable section | ||
| end Field | ||
|
|
||
| namespace ClaimA2 |
There was a problem hiding this comment.
Same comments about the fact instances from here onwards. Do we want them as facts or shall we not just add them as hypotheses whenever needed?
|
|
||
| def γ' (x₀ : F) (R : F[X][X][Y]) (H_irreducible : Irreducible H) : PowerSeries (𝕃 H) := | ||
| /-- The power series `γ'` with bundled irreducibility witness. -/ | ||
| noncomputable def γ' (x₀ : F) (R : F[X][X][Y]) (H_irreducible : Irreducible H) : PowerSeries (𝕃 H) := |
There was a problem hiding this comment.
Weird that here we have irreducibility of H as a hypothesis rather than a fact. We should probably have it one way or the other to be consistent with our conventions
|
/review External: Internal: Comments: |
🤖 AI Review (with external context)🤖 AI ReviewOverall Summary: 📄 **Review for `ArkLib/Data/Polynomial/RationalFunctions.lean`**Verdict: Major Issues Found The formalization contains a critical issue in the definition of the Hensel lift, rendering the constructed power series Critical Issue: Incorrect Definition of Hensel Lift Coefficients ( theorem β_regular ... : ... ∃ β : 𝒪 H, ... := by
intro t
refine ⟨(0 : 𝒪 H), ?_⟩
...The proof is supplied by providing noncomputable def β (R : F[X][X][Y]) (t : ℕ) : 𝒪 H :=
(β_regular R H ... t).chooseSince the proof of Impact: Correction Strategy: Other Findings:
Summary: |
No description provided.