Skip to content

refactor(RingTheory/Ideal/Heoght): minimize usages of Ideal.primeHeight#37500

Open
Thmoas-Guan wants to merge 24 commits intoleanprover-community:masterfrom
Thmoas-Guan:minimalPrimes-API
Open

refactor(RingTheory/Ideal/Heoght): minimize usages of Ideal.primeHeight#37500
Thmoas-Guan wants to merge 24 commits intoleanprover-community:masterfrom
Thmoas-Guan:minimalPrimes-API

Conversation

@Thmoas-Guan
Copy link
Copy Markdown
Collaborator

@Thmoas-Guan Thmoas-Guan commented Apr 1, 2026

deprecate and private most of primeHeight APIs, providing height variant.


Open in Gitpod

@github-actions
Copy link
Copy Markdown

github-actions bot commented Apr 1, 2026

PR summary 2ff88851d5

Import changes for modified files

No significant changes to the import graph

Import changes for all files
Files Import difference

Declarations diff

+ Ideal.height_add_one_le_of_lt_of_isPrime
+ Ideal.height_eq_ringKrullDim_iff
+ Ideal.height_eq_zero_iff
+ Ideal.height_strict_mono_of_isPrime
+ Ideal.height_strict_mono_of_isPrime_of_isPrime
+ Ideal.isMaximal_of_height_eq_ringKrullDim
+ Ideal.le_minimalPrimes
+ Ideal.sup_height_isPrime_eq_ringKrullDim
+ Ideal.sup_height_of_maximal_eq_ringKrullDim
+ minimalPrimes_isPrime
- Ideal.height_strict_mono_of_is_prime

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.


No changes to technical debt.

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 github-actions bot added the t-ring-theory Ring theory label Apr 1, 2026
@erdOne
Copy link
Copy Markdown
Member

erdOne commented Apr 1, 2026

If you are doing this separately maybe you should also go around and fix the current violations?

@Thmoas-Guan
Copy link
Copy Markdown
Collaborator Author

Sorry, what do you mean?

@erdOne
Copy link
Copy Markdown
Member

erdOne commented Apr 1, 2026

Can you replace the hI.1.1's throughout mathlib with the new lemmas?
Can you go around the library and remove the mentions of primeHeight whenever inappropriate?
Thanks.

@Thmoas-Guan
Copy link
Copy Markdown
Collaborator Author

Except for strict monotonicity, I added all height analogus and if possible use it to imply the primeHeight verison, up to you to decide which to depreceate and which mark private.
I only know mem_minimalPrimes_of_primeHeight_eq_height is ready to deprecate?

We still have abuse of hp.1.2, I'll fix it tomorrow.

@Thmoas-Guan
Copy link
Copy Markdown
Collaborator Author

Thmoas-Guan commented Apr 2, 2026

@erdOne I added Ideal.le_minimalPrimes for hp.1.2, what do you think now? I privated or deprecated all primeHeight related stuffs except for the two strict monotonicity lemmas.

@Thmoas-Guan Thmoas-Guan changed the title feat(RingTheory/minimalPrimes): add some lemmas for minimalPrimes refactor(RingTheory/Ideal/Heoght): minimize usages of Ideal.primeHeight and add lemma for minimalPrimes Apr 3, 2026
@Thmoas-Guan Thmoas-Guan changed the title refactor(RingTheory/Ideal/Heoght): minimize usages of Ideal.primeHeight and add lemma for minimalPrimes refactor(RingTheory/Ideal/Heoght): minimize usages of Ideal.primeHeight Apr 14, 2026
@mathlib-dependent-issues mathlib-dependent-issues bot added the blocked-by-other-PR This PR depends on another PR (this label is automatically managed by a bot) label Apr 14, 2026
@mathlib-dependent-issues
Copy link
Copy Markdown

This PR/issue depends on:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

blocked-by-other-PR This PR depends on another PR (this label is automatically managed by a bot) t-ring-theory Ring theory

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants