Skip to content

Conversation

ADA-Projects
Copy link

@ADA-Projects ADA-Projects commented Sep 16, 2025

This PR extends topological Krull dimension theory with basic results about subspaces.

  • Add mapIrreducibleClosed function for canonical maps between irreducible closed sets
  • Prove injectivity and strict monotonicity of these maps
  • Establish partial order structure on IrreducibleCloseds
  • Prove topKrullDim_subspace_le theorem showing dim(Y) ≤ dim(X) for subspaces

Update (per reviewer feedback):

Reorganized code structure as requested:

  • Moved IrreducibleCloseds extensions from KrullDimension.lean to Sets/Closeds.lean
  • This includes PartialOrder instance, map functions, and embedding lemmas
  • Better modularity: other files can now use IrreducibleCloseds without importing Krull dimension theory

The main result topologicalKrullDim_subspace_le remains the same - any subspace Y has dim(Y) ≤ dim(X).

Some sections of this code (and documentation) were generated with the help of Gemini.

Update 2

Addressed reviewer feedback with the following improvements:

  • Simplify IrreducibleCloseds.map to work with any continuous function
    (no longer requires closed maps)
  • Add coe_map lemma
  • Rename lemmas from mapOfEmbedding_* to map_*_of_isEmbedding for
    consistency with Mathlib naming conventions
  • Use inferInstance for PartialOrder instance on IrreducibleCloseds
  • Merge variable declarations in Sets/Closeds.lean
  • Add necessary imports (Topology.Defs.Induced)
  • Open Topology namespace for cleaner code
  • Remove unnecessary wrappers in 'Sets/Closeds.lean'

Update 3

Addressed and integrated reviewer feedback:

  • Use IsInducing instead of IsEmbedding in dimension theorems
  • Rename map_of_isEmbedding to map_of_isInducing
  • Simplify map_injective_of_isInducing proof using SetLike.coe_injective
  • Simplify map_strictMono_of_isInducing using Monotone.strictMono_of_injective
  • Add deprecation alias for IsClosedEmbedding.topologicalKrullDim_le
  • Remove redundant PartialOrder instance (already inferred)

The main theorem topologicalKrullDim_subspace_le remains unchanged.


Open in Gitpod

@github-actions github-actions bot added new-contributor This PR was made by a contributor with at most 5 merged PRs. Welcome to the community! t-topology Topological spaces, uniform spaces, metric spaces, filters labels Sep 16, 2025
Copy link

github-actions bot commented Sep 16, 2025

PR summary 1838dfbf5d

Import changes for modified files

No significant changes to the import graph

Import changes for all files
Files Import difference

Declarations diff

+ IsInducing.topologicalKrullDim_le
+ coe_map
+ map
+ map_injective_of_isInducing
+ map_mono
+ map_strictMono_of_isInducing
+ topologicalKrullDim_subspace_le
- IrreducibleCloseds.map
- IrreducibleCloseds.map_strictMono

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

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

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


No changes to technical debt.

You can run this locally as

./scripts/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).

@kim-em kim-em added the awaiting-author A reviewer has asked the author a question or requested changes. label Sep 17, 2025
@fpvandoorn
Copy link
Member

If you think you have addressed all the review comments, please remove the label awaiting-author by writing a comment with the text -awaiting-author.

@ADA-Projects
Copy link
Author

-awaiting-author

@github-actions github-actions bot removed the awaiting-author A reviewer has asked the author a question or requested changes. label Sep 25, 2025
@erdOne erdOne added the awaiting-author A reviewer has asked the author a question or requested changes. label Oct 14, 2025
@erdOne erdOne assigned erdOne and unassigned PatrickMassot Oct 14, 2025
@mathlib4-merge-conflict-bot mathlib4-merge-conflict-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 Oct 15, 2025
@mathlib4-merge-conflict-bot
Copy link
Collaborator

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

@github-actions github-actions bot removed the merge-conflict The PR has a merge conflict with master, and needs manual merging. (this label is managed by a bot) label Oct 18, 2025
@ADA-Projects
Copy link
Author

-awaiting-author

@github-actions github-actions bot removed the awaiting-author A reviewer has asked the author a question or requested changes. label Oct 18, 2025
@erdOne
Copy link
Member

erdOne commented Oct 19, 2025

I think you have messed up something. Can you try merging master again?

ADA-Projects and others added 5 commits October 19, 2025 11:39
  dimension inequality

  • Add mapIrreducibleClosed function for
  canonical maps between irreducible closed
  sets
  • Prove injectivity and strict monotonicity
  of these maps
  • Establish partial order structure on
  IrreducibleCloseds
  • Prove topKrullDim_subspace_le theorem
  showing dim(Y) ≤ dim(X) for subspaces
…nsion to Sets/Closeds

Move PartialOrder instance, map functions, and embedding-related lemmas from
Topology/KrullDimension.lean to Topology/Sets/Closeds.lean for better code organization.
This separates basic IrreducibleCloseds operations from topological Krull dimension theory,
allowing other modules to use IrreducibleCloseds functionality without importing
dimension-specific concepts.

Changes:
- Move PartialOrder instance and le_iff_subset/lt_iff_subset lemmas to Sets/Closeds
- Move IrreducibleCloseds.map and related functions for closed continuous maps
- Move mapOfEmbedding functions and lemmas for general embeddings
- Move mapIrreducibleClosed functions for subspace embeddings
- Keep only topological Krull dimension definitions and theorems in KrullDimension
- Add necessary imports to Sets/Closeds for embedding support
Co-authored-by: Johan Commelin <johan@commelin.net>
…anization

Address reviewer feedback with the following improvements:

- Simplify `IrreducibleCloseds.map` to work with any continuous function
  (no longer requires closed maps)
- Rename lemmas from `mapOfEmbedding_*` to `map_*_of_embedding` for
  consistency with Mathlib naming conventions
- Use `inferInstance` for PartialOrder instance on IrreducibleCloseds
- Merge variable declarations in Sets/Closeds.lean
- Add necessary imports (Topology.Defs.Induced)
- Open Topology namespace for cleaner code
- Simplify proofs and improve readability

The main theorem `topologicalKrullDim_subspace_le` remains unchanged.
…g consistency

- Simplify readability in `IrreducibleCloseds.map` making the function explicit
- Rename `map_*_of_embedding` to `map_*_of_isEmbedding` for Mathlib consistency
- Add `coe_map` simp lemma for cleaner proofs
- Remove redundant `mapIrreducibleClosed` wrapper functions
  dimension inequality

  • Add mapIrreducibleClosed function for
  canonical maps between irreducible closed
  sets
  • Prove injectivity and strict monotonicity
  of these maps
  • Establish partial order structure on
  IrreducibleCloseds
  • Prove topKrullDim_subspace_le theorem
  showing dim(Y) ≤ dim(X) for subspaces
- Rename map_*_of_isEmbedding to map_*_of_isInducing for generality
- Simplify map_injective_of_isInducing proof using SetLike.coe_injective
- Simplify map_strictMono_of_isInducing using Monotone.strictMono_of_injective
- Use IsInducing instead of IsEmbedding in dimension theorems
- Add deprecation alias for IsClosedEmbedding.topologicalKrullDim_le
- Remove redundant PartialOrder instance (already inferred)
Copy link
Member

@erdOne erdOne left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the great PR and for the patience!
maintainer delegate

Copy link

🚀 Pull request has been placed on the maintainer queue by erdOne.

@leanprover-community-mathlib4-bot leanprover-community-mathlib4-bot added the maintainer-merge A reviewer has approved the changed; awaiting maintainer approval. label Oct 20, 2025
@ADA-Projects
Copy link
Author

I've done 'lake build Mathlib' and everything works fine in my fork. Why did the CI test fail? it seems the cache of the files I've modified is missing.

@erdOne
Copy link
Member

erdOne commented Oct 21, 2025

Can you try merging master? It might be an issue about a change in CI.

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

Labels

maintainer-merge A reviewer has approved the changed; awaiting maintainer approval. new-contributor This PR was made by a contributor with at most 5 merged PRs. Welcome to the community! t-topology Topological spaces, uniform spaces, metric spaces, filters

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants