Skip to content

experiment: rm_set_option.py after greedily deploying #35950's inferInstanceAs%#36573

Closed
kim-em wants to merge 15 commits intomasterfrom
how_many_35950_rm
Closed

experiment: rm_set_option.py after greedily deploying #35950's inferInstanceAs%#36573
kim-em wants to merge 15 commits intomasterfrom
how_many_35950_rm

Conversation

@kim-em
Copy link
Copy Markdown
Contributor

@kim-em kim-em commented Mar 13, 2026

Removes 769 set_option backwards.

kim-em and others added 15 commits March 12, 2026 12:09
…ed instances

`inferInstanceAs%` is a drop-in replacement for `inferInstanceAs` that
prevents "type leakage" in synthesized instances. When `inferInstanceAs`
synthesizes an instance for a type alias, the resulting expression may
contain lambda binder domains referring to unfolded forms of the carrier
type rather than the declared alias. This is invisible at `default`
transparency but causes `isDefEq` failures at `reducibleAndInstances`
transparency — which is the level used by `grind`'s `checkInst`.

The fix recursively normalizes the constructor tree: for each
class-valued structure, it WHNFs to expose the constructor, patches the
carrier type parameter via `isDefEq` matching, recursively processes
instance-implicit fields, and replaces lambda binder domains in function
fields.

This fixes the `grind` failure in `FiniteResidueField` that was
previously worked around with an `#adaptation_note` and manual proof.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Refactor the implementation:
- Make `unfoldChain` iterative with built-in dedup and `skipHead` param
- Extract `addUnfoldings` and `buildReplacements` from inline logic
- Split `normalizeInstance` into `getCtorApp?`, `getFieldInfo`, and
  `normalizeCtorArgs` helpers (mutual block for recursion)

Add deeper test hierarchy (TestInv → TestDivInvMonoid → TestField)
reproducing the real grind failure pattern, with declarative examples
showing defeq at default transparency but not at `instances` transparency.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Use parallel `for sArg in sourceArgs, eArg in expectedArgs` (#2)
- Use `repeat do` instead of fuel-bounded loop (#3)
- Use modern range syntax `*...n` and `n...m` (#9, #10)
- Use `fieldInfo[i]?` instead of bounds check (#11)

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Use `.getD ty` instead of matching on `matchesAnyDefeq` result (#5)
- Remove unnecessary `withDefault` on `isProp` (#6)
- Move `mkAppN` into `normalizeCtorArgs`, returning `Expr` directly (#12)

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Remove extra leading spaces on continuation lines of docstrings
to match mathlib convention (#8).

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…ard getAppFn

Throw an error when source and expected types fail to unify, and
check that the class heads match before comparing arguments pairwise (#1).

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
All code is `meta`, so `partial` is fine for termination. Remove the
unnecessary fuel parameter from `normalizeCtorArgs` and `normalizeInstance`.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…add option

When `inferInstanceAs%` patches an instance, it now tries to synthesize
sub-instances for the target carrier type. If a synthesized sub-instance
is not defeq to the patched version at `reducibleAndInstances` transparency,
a warning is emitted suggesting the user define it separately.

The warning can be disabled with
`set_option inferInstanceAsPercent.leakySubInstWarning false`.

Also regenerates import files via `lake exe mk_all`.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
When a sub-instance was already defined with `inferInstanceAs%`, the
leaky sub-instance warning would still fire because the synthesized
sub-instance differed from the freshly-patched version (different
construction paths in the hierarchy). Now we check if `synthInst` is
itself clean (normalizing it doesn't change it) before warning.

Also improves the warning message to explain what's happening and
how to suppress it.

Reported by Sébastien Gouëzel.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…abbreviations

The head comparison used structural equality (`==`) on `Expr`, which
fails when the same constant appears at different universe levels
(e.g. `DecidableEq.{u+1}` vs `DecidableEq.{max 1 (u+1)}`) or when
source and expected types use different abbreviations (e.g.
`DecidableLT` vs `DecidableRel`).

Replace the strict check with `alignHeads`, which:
- compares only constant names (ignoring universe levels)
- tries unfolding both types to find a common head constant

Reported by Sébastien Gouëzel.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Previously, `normalizeCtorArgs` called `trySynthInstance` for every
instance-implicit field at every level of the class hierarchy. For
deep hierarchies like `Field`, this cascaded into ~30-40 expensive
synthesis calls.

Now only the top-level constructor's fields trigger synthesis
(`trySynth = true`). Recursive normalization uses purely mechanical
patching (`trySynth = false`): WHNF + carrier replacement + lambda
domain patching, with no synthesis overhead.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Remove `addUnfoldings` and simplify `buildReplacements` to just collect
differing arg pairs — `matchesAnyDefeq` already uses `isDefEq` at
`default` transparency, so explicit unfold chains are redundant.

Rename `getCtorApp?` to `constructorAppWithUniverses?` referencing
Lean core's `constructorApp?`.

Extract `processInstFieldWithSynth` from the deeply nested trySynth
logic in `normalizeCtorArgs`, and flatten the field loop with `continue`.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@github-actions github-actions bot added the large-import Automatically added label for PRs with a significant increase in transitive imports label Mar 13, 2026
@github-actions
Copy link
Copy Markdown

github-actions bot commented Mar 13, 2026

PR summary 95b694b853

Import changes exceeding 2%

% File
+2.04% Mathlib.Init

Import changes for modified files

No significant changes to the import graph

Import changes for all files
Files Import difference
There are 7732 files with changed transitive imports taking up over 334481 characters: this is too many to display!
You can run ci-tools/scripts/pr_summary/import_trans_difference.sh all locally to see the whole output.

Declarations diff

+ B
+ MyInv
+ MyNat
+ MyVec
+ TestAbbrevRel
+ TestBaseRel
+ TestDivInvMonoid
+ TestField
+ TestInv
+ TestNat
+ alignHeads
+ buildReplacements
+ constructorAppWithUniverses?
+ getFieldInfo
+ instance (priority := low) : (ker f).IsTwoSided := inferInstanceAs% (Ideal.comap f ⊥).IsTwoSided
+ instance (x : X) : Unique (Spec (X.residueField x)) := inferInstanceAs% (Unique (Spec <| .of _))
+ instance : (refl (C := C)).functor.Monoidal := inferInstanceAs% (𝟭 C).Monoidal
+ instance : (refl (C := C)).inverse.Monoidal := inferInstanceAs% (𝟭 C).Monoidal
+ instance : (restrictScalars (𝟙 R)).Full := inferInstanceAs% (𝟭 _).Full
+ instance : Abelian (Presheaf C X) := inferInstanceAs% (Abelian (_ ⥤ _))
+ instance : AddCommGroup (TrivialLieModule R L M) := inferInstanceAs% (AddCommGroup M)
+ instance : AddCommGroup P.Cotangent := inferInstanceAs% (AddCommGroup P.ker.Cotangent)
+ instance : Algebra S T := inferInstanceAs% (Algebra S (integralClosure S E))
+ instance : Category (Bimon C) := inferInstanceAs% (Category (Comon (Mon C)))
+ instance : Category (CatEnrichedOrdinary C) := inferInstanceAs% (Category C)
+ instance : Category (TransportEnrichment F C) := inferInstanceAs% (Category C)
+ instance : Category t.Category := inferInstanceAs% (Category C)
+ instance : CommRing (WithPreorder R) := inferInstanceAs% (CommRing R)
+ instance : CommRing T := inferInstanceAs% (CommRing (integralClosure S E))
+ instance : DiscreteTopology ℕ+ := inferInstanceAs% (DiscreteTopology { n : ℕ // 0 < n })
+ instance : E.symm.functor.CommShift A := inferInstanceAs% (E.inverse.CommShift A)
+ instance : E.symm.inverse.CommShift A := inferInstanceAs% (E.functor.CommShift A)
+ instance : EnrichedCategory Cat (CatEnriched C) := inferInstanceAs% (EnrichedCategory Cat C)
+ instance : EnrichedCategory Cat (CatEnrichedOrdinary C) := inferInstanceAs% (EnrichedCategory Cat C)
+ instance : Field (FiniteResidueField K)
+ instance : Field (⊤ : ValuationSubring K) := inferInstanceAs% (Field (⊤ : Subfield K))
+ instance : HasColimits X.Modules := inferInstanceAs% (HasColimits (SheafOfModules X.ringCatSheaf))
+ instance : HasLimits X.Modules := inferInstanceAs% (HasLimits (SheafOfModules X.ringCatSheaf))
+ instance : IsDomain (ZMod 0) := inferInstanceAs% (IsDomain ℤ)
+ instance : IsDomain T := inferInstanceAs% (IsDomain (integralClosure S E))
+ instance : IsOpenImmersion U.ι := inferInstanceAs% (IsOpenImmersion (X.ofRestrict _))
+ instance : MetricSpace ℕ+ := inferInstanceAs% (MetricSpace { n : ℕ // 0 < n })
+ instance : Module R (TrivialLieModule R L M) := inferInstanceAs% (Module R M)
+ instance : MyInv Nat
+ instance : Nontrivial T := inferInstanceAs% (Nontrivial (integralClosure S E))
+ instance : SomeClass B := inferInstanceAs (SomeClass A)
+ instance : TestAbbrevRel := inferInstanceAs% (TestBaseRel (· < ·))
+ instance : TestBaseRel (· < ·)
+ instance : TestField Nat
+ instance : inclusion.Faithful := inferInstanceAs% WithInitial.incl.Faithful
+ instance : inclusion.Full := inferInstanceAs% WithInitial.incl.Full
+ instance [DecidableEq α] : DecidableEq (MyVec α n)
+ instance [e.functor.Monoidal] : e.symm.inverse.Monoidal := inferInstanceAs% (e.functor.Monoidal)
+ instance [e.inverse.Monoidal] : e.symm.functor.Monoidal := inferInstanceAs% (e.inverse.Monoidal)
+ instance {E} [Semiring E] [Algebra L E] : Algebra S E := inferInstanceAs% (Algebra S.toSubalgebra E)
+ instance {M N : Comon C} (f : M ⟶ N) : IsComonHom f.hom := inferInstanceAs% (IsComonHom f.hom)
+ instance {X Y : CatEnriched C} : Category (X ⟶ Y) := inferInstanceAs% (Category (X ⟶[Cat] Y).α)
+ matchesAnyDefeq
+ myNatInv_fixed
+ myNatInv_leaky
+ replaceCarriersInType
+ sameHeadConstName
+ testField_direct
+ testField_fixed
+ testField_fixed'
+ testField_leaky
+ testField_with_clean_inv
+ testInv_clean
+ unfoldChain
++ instance : (L').IsLocalization W := inferInstanceAs% (L.IsLocalization W)
- instance (priority := low) : (ker f).IsTwoSided := inferInstanceAs (Ideal.comap f ⊥).IsTwoSided
- instance (x : X) : Unique (Spec (X.residueField x)) := inferInstanceAs (Unique (Spec <| .of _))
- instance : (refl (C := C)).functor.Monoidal := inferInstanceAs (𝟭 C).Monoidal
- instance : (refl (C := C)).inverse.Monoidal := inferInstanceAs (𝟭 C).Monoidal
- instance : (restrictScalars (𝟙 R)).Full := inferInstanceAs (𝟭 _).Full
- instance : Abelian (Presheaf C X) := inferInstanceAs (Abelian (_ ⥤ _))
- instance : AddCommGroup (TrivialLieModule R L M) := inferInstanceAs (AddCommGroup M)
- instance : AddCommGroup P.Cotangent := inferInstanceAs (AddCommGroup P.ker.Cotangent)
- instance : Algebra S T := inferInstanceAs (Algebra S (integralClosure S E))
- instance : Category (Bimon C) := inferInstanceAs (Category (Comon (Mon C)))
- instance : Category (CatEnrichedOrdinary C) := inferInstanceAs (Category C)
- instance : Category (TransportEnrichment F C) := inferInstanceAs (Category C)
- instance : Category t.Category := inferInstanceAs (Category C)
- instance : CommRing (WithPreorder R) := inferInstanceAs (CommRing R)
- instance : CommRing T := inferInstanceAs (CommRing (integralClosure S E))
- instance : DiscreteTopology ℕ+ := inferInstanceAs (DiscreteTopology { n : ℕ // 0 < n })
- instance : E.symm.functor.CommShift A := inferInstanceAs (E.inverse.CommShift A)
- instance : E.symm.inverse.CommShift A := inferInstanceAs (E.functor.CommShift A)
- instance : EnrichedCategory Cat (CatEnriched C) := inferInstanceAs (EnrichedCategory Cat C)
- instance : EnrichedCategory Cat (CatEnrichedOrdinary C) := inferInstanceAs (EnrichedCategory Cat C)
- instance : Field (⊤ : ValuationSubring K) := inferInstanceAs (Field (⊤ : Subfield K))
- instance : HasColimits X.Modules := inferInstanceAs (HasColimits (SheafOfModules X.ringCatSheaf))
- instance : HasLimits X.Modules := inferInstanceAs (HasLimits (SheafOfModules X.ringCatSheaf))
- instance : IsDomain (ZMod 0) := inferInstanceAs (IsDomain ℤ)
- instance : IsDomain T := inferInstanceAs (IsDomain (integralClosure S E))
- instance : IsOpenImmersion U.ι := inferInstanceAs (IsOpenImmersion (X.ofRestrict _))
- instance : MetricSpace ℕ+ := inferInstanceAs (MetricSpace { n : ℕ // 0 < n })
- instance : Module R (TrivialLieModule R L M) := inferInstanceAs (Module R M)
- instance : Nontrivial T := inferInstanceAs (Nontrivial (integralClosure S E))
- instance : inclusion.Faithful := inferInstanceAs WithInitial.incl.Faithful
- instance : inclusion.Full := inferInstanceAs WithInitial.incl.Full
- instance [e.functor.Monoidal] : e.symm.inverse.Monoidal := inferInstanceAs (e.functor.Monoidal)
- instance [e.inverse.Monoidal] : e.symm.functor.Monoidal := inferInstanceAs (e.inverse.Monoidal)
- instance {E} [Semiring E] [Algebra L E] : Algebra S E := inferInstanceAs (Algebra S.toSubalgebra E)
- instance {M N : Comon C} (f : M ⟶ N) : IsComonHom f.hom := inferInstanceAs (IsComonHom f.hom)
- instance {X Y : CatEnriched C} : Category (X ⟶ Y) := inferInstanceAs (Category (X ⟶[Cat] Y).α)
-- instance : (L').IsLocalization W := inferInstanceAs (L.IsLocalization W)

You can run this locally as follows
## summary with just the declaration names:
./scripts/pr_summary/declarations_diff.sh <optional_commit>

## more verbose report:
./scripts/pr_summary/declarations_diff.sh long <optional_commit>

The doc-module for scripts/pr_summary/declarations_diff.sh contains some details about this script.


Decrease in tech debt: (relative, absolute) = (11.24, 0.09)
Current number Change Type
127 -1 adaptation notes
9399 -769 backward.isDefEq

Current commit c0a1d99809
Reference commit 95b694b853

You can run this locally as

./scripts/reporting/technical-debt-metrics.sh pr_summary
  • The relative value is the weighted sum of the differences with weight given by the inverse of the current value of the statistic.
  • The absolute value is the relative value divided by the total sum of the inverses of the current values (i.e. the weighted average of the differences).

@github-actions
Copy link
Copy Markdown

🚨 PR Title Needs Formatting

Please update the title to match our commit style conventions.

Errors from script:

error: the PR title should be of the form
  kind: main title
or
  kind(scope): main title
Allowed values for `kind` are [feat, chore, perf, refactor, style, fix, doc, test, ci]
Details on the required title format

The title should fit the following format:

<kind>(<optional-scope>): <subject>

<kind> is:

  • feat (feature)
  • fix (bug fix)
  • doc (documentation)
  • style (formatting, missing semicolons, ...)
  • refactor
  • test (when adding missing tests)
  • chore (maintain)
  • perf (performance improvement, optimization, ...)
  • ci (changes to continuous integration, repo automation, ...)

<optional-scope> is a name of module or a directory which contains changed modules.
This is not necessary to include, but may be useful if the <subject> is insufficient.
The Mathlib directory prefix is always omitted.
For instance, it could be

  • Data/Nat/Basic
  • Algebra/Group/Defs
  • Topology/Constructions

<subject> has the following constraints:

  • do not capitalize the first letter
  • no dot(.) at the end
  • use imperative, present tense: "change" not "changed" nor "changes"

@mathlib-merge-conflicts
Copy link
Copy Markdown

This pull request has conflicts, please merge master and resolve them.

@mathlib-merge-conflicts mathlib-merge-conflicts bot added the merge-conflict The PR has a merge conflict with master, and needs manual merging. (this label is managed by a bot) label Mar 13, 2026
@kim-em kim-em closed this Apr 2, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

large-import Automatically added label for PRs with a significant increase in transitive imports merge-conflict The PR has a merge conflict with master, and needs manual merging. (this label is managed by a bot)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant