Skip to content

reverting the refactor#74

Open
dhsorens wants to merge 1 commit intomasterfrom
revert-72-dhsorens/canonical-refactor
Open

reverting the refactor#74
dhsorens wants to merge 1 commit intomasterfrom
revert-72-dhsorens/canonical-refactor

Conversation

@dhsorens
Copy link
Collaborator

@dhsorens dhsorens commented Feb 6, 2026

Reverts #72

The refactor ran us into some weird typeclass issues and for now it might be a good idea to revert back to previous file structure and then re-do the refactor more carefully. Opening this branch to have something to work from and hopefully resolve the typeclass issues.

@github-actions
Copy link

github-actions bot commented Feb 6, 2026

🤖 Gemini PR Summary

This Pull Request reverts a previous refactor (PR #72) to resolve significant typeclass resolution issues. The primary focus of this change is the reorganization of the univariate polynomial hierarchy, specifically regarding how "raw" and "canonical" polynomials are represented and named.

Refactoring

  • Type Renaming & Namespace Consolidation:
    • Renamed the previous CPolynomial.Raw (the base array-based representation) to CPolynomial.
    • Renamed the previous CPolynomial (the subtype representing canonical polynomials with no trailing zeros) to CPolynomialC.
    • Deleted CompPoly/Univariate/Raw.lean, merging its core logic into the primary Basic.lean file.
  • Module Synchronization: Updated references across the library to reflect the new naming convention, specifically affecting:
    • Equiv.lean: Equivalence proofs between computable and mathlib polynomials.
    • Lagrange.lean: Lagrange interpolation logic.
    • Quotient.lean: Polynomial quotient definitions and proofs.
  • Import Optimization: Updated CompPoly.lean to reflect the transition from raw representations to the canonical CPolynomialC imports where appropriate.

Fixes

  • Typeclass Resolution: Addresses "weird typeclass issues" introduced in the previous iteration by reverting to a simpler organizational structure. This ensures that algebraic instances (Groups, Rings, etc.) for polynomials are correctly synthesized by the Lean compiler.

Features

  • Array-based Polynomial Implementation: Re-implements CPolynomial R as an Array R within Basic.lean, including a comprehensive suite of computable operations, trimming logic, and associated algebraic properties.
  • Canonical Subtype Definition: Established CPolynomialC in Canonical.lean as the canonical form of univariate polynomials, ensuring a unique representation for algebraic structures.

Analysis of Changes

Metric Count
📝 Files Changed 7
Lines Added 1759
Lines Removed 1723

sorry Tracking

✅ **Removed:** 12 `sorry`(s)
  • instance [CommSemiring R] [LawfulBEq R] [Nontrivial R] : CommSemiring (CPolynomial R) where in CompPoly/Univariate/Basic.lean
  • lemma mul_assoc (p q r : CPolynomial R) : (p * q) * r = p * (q * r) in CompPoly/Univariate/Basic.lean
  • lemma mul_one (p : CPolynomial R) : p * 1 = p in CompPoly/Univariate/Basic.lean
  • lemma left_distrib (p q r : CPolynomial R) : p * (q + r) = p * q + p * r in CompPoly/Univariate/Basic.lean
  • lemma right_distrib (p q r : CPolynomial R) : (p + q) * r = p * r + q * r in CompPoly/Univariate/Basic.lean
  • def monomialC [DecidableEq R] (n : ℕ) (c : R) : CPolynomial R in CompPoly/Univariate/Basic.lean
  • lemma mul_zero (p : CPolynomial R) : p * 0 = 0 in CompPoly/Univariate/Basic.lean
  • instance [Semiring R] [LawfulBEq R] [Nontrivial R] : Semiring (CPolynomial R) where in CompPoly/Univariate/Basic.lean
  • lemma zero_mul (p : CPolynomial R) : 0 * p = 0 in CompPoly/Univariate/Basic.lean
  • instance : Mul (CPolynomial R) where in CompPoly/Univariate/Basic.lean
  • instance [Ring R] [LawfulBEq R] [Nontrivial R] : Ring (CPolynomial R) where in CompPoly/Univariate/Basic.lean
  • lemma one_mul (p : CPolynomial R) : 1 * p = p in CompPoly/Univariate/Basic.lean
❌ **Added:** 12 `sorry`(s)
  • instance [CommSemiring R] [LawfulBEq R] [Nontrivial R] : CommSemiring (CPolynomialC R) where in CompPoly/Univariate/Canonical.lean
  • lemma left_distrib (p q r : CPolynomialC R) : p * (q + r) = p * q + p * r in CompPoly/Univariate/Canonical.lean
  • instance [Ring R] [LawfulBEq R] [Nontrivial R] : Ring (CPolynomialC R) where in CompPoly/Univariate/Canonical.lean
  • instance [Semiring R] [LawfulBEq R] [Nontrivial R] : Semiring (CPolynomialC R) where in CompPoly/Univariate/Canonical.lean
  • lemma zero_mul (p : CPolynomialC R) : 0 * p = 0 in CompPoly/Univariate/Canonical.lean
  • lemma right_distrib (p q r : CPolynomialC R) : (p + q) * r = p * r + q * r in CompPoly/Univariate/Canonical.lean
  • lemma mul_assoc (p q r : CPolynomialC R) : (p * q) * r = p * (q * r) in CompPoly/Univariate/Canonical.lean
  • instance : Mul (CPolynomialC R) where in CompPoly/Univariate/Canonical.lean
  • lemma mul_one (p : CPolynomialC R) : p * 1 = p in CompPoly/Univariate/Canonical.lean
  • lemma mul_zero (p : CPolynomialC R) : p * 0 = 0 in CompPoly/Univariate/Canonical.lean
  • lemma one_mul (p : CPolynomialC R) : 1 * p = p in CompPoly/Univariate/Canonical.lean
  • def monomialC [DecidableEq R] (n : ℕ) (c : R) : CPolynomialC R in CompPoly/Univariate/Canonical.lean

🎨 **Style Guide Adherence**

Based on the provided style guide, here are the violations identified in the code changes:

CompPoly/Univariate/Basic.lean

  • Line 29, 36, 40, 311, 446: def CPolynomial (R : Type*) := Array R, def mk ..., def coeffs ..., def natDegreeBound ..., def canonical ...
    • Violation: "Every definition and major theorem should have a docstring."
  • Line 84, 114: (∀ i, (hi : i < p.size) → p[i] = 0) → p.lastNonzero = none := by, lastNonzeroProp k → lastNonzeroProp k' → k = k' := by
    • Violation: "Hypotheses: Prefer placing hypotheses to the left of the colon (e.g., (h : P) : Q) rather than using arrows (: P → Q) when the proof introduces them."
  • Line 271, 298, 300, 301 (and throughout): (fun acc ⟨a, i⟩ => acc + f a * x ^ i), (fun a => r * a), etc.
    • Violation: "Functions: Prefer fun x ↦ ... over λ x, ...." (The code uses the => symbol instead of the specified ).
  • Lines 302-310: instance : Zero (CPolynomial R) := ⟨#[]⟩, instance : One (CPolynomial R) := ⟨CPolynomial.C 1⟩, etc.
    • Violation: "Instances: Use the where syntax for defining instances and structures."
  • Line 317, 446: def monic (p : CPolynomial R) : Bool, def canonical (p : CPolynomial R)
    • Violation: "Predicates: Generally use prefixes (e.g., isClosed_Icc not Icc_isClosed)." (Should be isMonic and isCanonical).
  • Lines 364-367: p(x) = a_0 + a_1 x + a_2 x^2 + ... + a_n x^n
    • Violation: "Syntax: ... Use LaTeX for math: $ f(x) = y $ (inline) or $$ \sum_{i=0}^n i $$ (display)."
  • Line 441, 815, 879, 915 (and throughout): theorem add_comm : p + q = q + p := by, protected theorem mul_assoc [LawfulBEq R] (p q r : CPolynomial R) :
    • Violation: "Variable Conventions: ... x, y, z, ... : Elements of a generic type; p, q, r, ... : Predicates and relations" (The code uses p, q, and r as variables for polynomial elements).
  • Line 511: = z.coeff k + (l.map (fun x => (f x).coeff k)).sum := by
    • Violation: "Operators: ... Place them before a line break rather than at the start of the next line."
  • Line 612: ((smul a q).coeff i) + ((smul a r).coeff i) =
    • Violation: "Indentation: Use 2 spaces for indentation." (Excessive whitespace used to align terms).

CompPoly/Univariate/Canonical.lean

  • Line 34: def CPolynomialC (R : Type*) [BEq R] [Ring R] := { p : CPolynomial R // p.trim = p }
    • Violation: "Every definition and major theorem should have a docstring."
  • Line 54, 76, 85, 114 (and throughout): instance : Add (CPolynomialC R) where, def nsmul (n : ℕ) (p : CPolynomialC R) : CPolynomialC R :=, etc.
    • Violation: "Variable Conventions: ... p, q, r, ... : Predicates and relations" (Used as elements).

CompPoly/Univariate/Equiv.lean

  • Line 31, 42, 59: def Polynomial.toImpl ..., noncomputable def toPoly ..., noncomputable def ringEquiv ...
    • Violation: "Every definition and major theorem should have a docstring."
  • Line 306: (0 : CPolynomial.Raw R).toPoly = 0 := by simp [toPoly, eval₂]
    • Violation: "Variable Conventions: ... p, q, r, ... : Predicates and relations" (Used as elements).

📄 **Per-File Summaries**
  • CompPoly.lean: Updates the univariate polynomial imports by replacing the raw representation with the canonical one.
  • CompPoly/Univariate/Basic.lean: Redefines CPolynomial R as an Array R and implements a comprehensive suite of computable polynomial operations, trimming logic, and their corresponding algebraic properties.
  • CompPoly/Univariate/Canonical.lean: This file defines CPolynomialC, a subtype for canonical univariate polynomials with no trailing zeros, and establishes their core algebraic structures, including group and ring instances.
  • CompPoly/Univariate/Equiv.lean: Refactors the equivalence proofs between computable and mathlib polynomials by renaming CPolynomial.Raw to CPolynomial and the canonical subtype CPolynomial to CPolynomialC.
  • CompPoly/Univariate/Lagrange.lean: Refactor Lagrange interpolation to use the CPolynomial type and namespace instead of CPolynomial.Raw.
  • CompPoly/Univariate/Quotient.lean: Renames the underlying polynomial type from CPolynomial.Raw to CPolynomial throughout the quotient definitions and proofs.
  • CompPoly/Univariate/Raw.lean: Deleted the implementation of raw computable univariate polynomials and their associated algebraic properties.

Last updated: 2026-02-06 09:55 UTC.

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.

1 participant