Skip to content

[Merged by Bors] - perf: make Field input to ClassGroup.mk explicit#39685

Closed
kbuzzard wants to merge 3 commits into
leanprover-community:masterfrom
kbuzzard:kbuzzard-classgroup-explicit-field
Closed

[Merged by Bors] - perf: make Field input to ClassGroup.mk explicit#39685
kbuzzard wants to merge 3 commits into
leanprover-community:masterfrom
kbuzzard:kbuzzard-classgroup-explicit-field

Conversation

@kbuzzard
Copy link
Copy Markdown
Member

@kbuzzard kbuzzard commented May 22, 2026

I noticed typeclass inference being slow to infer this input, so let's try making it explicit.

This is a way of removing the (W := W) hack introduced in #39124 .


Open in Gitpod

kbuzzard and others added 3 commits May 22, 2026 08:51
Passes the field of fractions `K` explicitly at every `ClassGroup.mk`
application, following the previous commit that made `K` an explicit
argument.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@github-actions
Copy link
Copy Markdown

PR summary 0b683b8515

Import changes for modified files

No significant changes to the import graph

Import changes for all files
Files Import difference

Declarations diff

No declarations were harmed in the making of this PR! 🐙

You can run this locally as follows
## from your `mathlib4` directory:
git clone https://github.com/leanprover-community/mathlib-ci.git ../mathlib-ci

## summary with just the declaration names:
../mathlib-ci/scripts/pr_summary/declarations_diff.sh <optional_commit>

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

The doc-module for scripts/pr_summary/declarations_diff.sh in the mathlib-ci repository contains some details about this script.


No changes to strong technical debt.
No changes to weak technical debt.

@github-actions github-actions Bot added the t-algebraic-geometry Algebraic geometry label May 22, 2026
@kbuzzard
Copy link
Copy Markdown
Member Author

!radar

@leanprover-radar
Copy link
Copy Markdown

leanprover-radar commented May 22, 2026

Benchmark results for 0b683b8 against c669471 are in. No significant results found. @kbuzzard

  • build//instructions: -13.9G (-0.01%)

Small changes (1✅, 2🟥)

  • build/module/Batteries.Data.BitVec//instructions: -20.3M (-1.42%)
  • 🟥 build/module/Mathlib.Analysis.Meromorphic.IsolatedZeros//instructions: +297.3M (+3.60%)
  • 🟥 build/module/Mathlib.Lean.Name//instructions: +174.8M (+5.57%)

@kbuzzard
Copy link
Copy Markdown
Member Author

kbuzzard commented May 22, 2026

The 14G decrease in instructions is little more than noise -- the point of this PR is that #39124 and #mathlib4 > Field (FunctionField ?m.19) @ 💬 shows that we have a footgun and this PR is an attempt to make it less likely to go off. Note that if you fill in the now-explicit input field with _ then it can still fire. The bottom line is that this

ClassGroup.mk.{u_1, u_2} {R : Type u_1} {K : Type u_2} [CommRing R] [Field K] [Algebra R K] [IsFractionRing R K]
  [IsDomain R] : (FractionalIdeal R⁰ K)ˣ →* ClassGroup R

is not great in Lean 4, and making K explicit is a bit better.

@mattrobball
Copy link
Copy Markdown
Contributor

bors merge

@mathlib-triage mathlib-triage Bot added the ready-to-merge This PR has been sent to bors. label May 22, 2026
mathlib-bors Bot pushed a commit that referenced this pull request May 22, 2026
I noticed typeclass inference being slow to infer this input, so let's try making it explicit.

This is a way of removing the `(W := W)` hack introduced in #39124 .
@mathlib-bors
Copy link
Copy Markdown
Contributor

mathlib-bors Bot commented May 22, 2026

Pull request successfully merged into master.

Build succeeded:

@mathlib-bors mathlib-bors Bot changed the title perf: make Field input to ClassGroup.mk explicit [Merged by Bors] - perf: make Field input to ClassGroup.mk explicit May 22, 2026
@mathlib-bors mathlib-bors Bot closed this May 22, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ready-to-merge This PR has been sent to bors. t-algebraic-geometry Algebraic geometry

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants