Skip to content

Conversation

YaelDillies
Copy link
Collaborator

@YaelDillies YaelDillies commented Nov 25, 2024

@github-actions github-actions bot added the t-algebra Algebra (groups, rings, fields, etc) label Nov 25, 2024
Copy link

github-actions bot commented Nov 25, 2024

PR summary 0231ea944e

Import changes for modified files

No significant changes to the import graph

Import changes for all files
Files Import difference
Mathlib.Geometry.Group.Marking (new file) 1530

Declarations diff

+ AddGroupMarking
+ GroupMarking
+ GroupMarking.refl
+ MarkedGroup
+ MarkedGroup.instGroup
+ MarkedGroup.instInhabited
+ MarkedGroup.instMulAction
+ MarkedGroup.instSmul
+ Subgroup.continuousConstSMul
+ Submonoid.continuousConstSMul
+ coe_surjective
+ instFunLike
+ instMonoidHomClass
+ instance : Inhabited (GroupMarking G G) := ⟨.refl⟩
+ instance : NormedGroup (MarkedGroup m)
+ norm_le_iff
+ ofMarkedGroup
+ ofMarkedGroup_inj
+ ofMarkedGroup_smul
+ ofMarkedGroup_symm_eq
+ ofMarkedGroup_toMarkedGroup
+ quotientEquivMarkedGroup
+ toMarkedGroup
+ toMarkedGroup_inj
+ toMarkedGroup_ofMarkedGroup
+ toMarkedGroup_smul
+ toMarkedGroup_symm_eq

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

/-!
# Marked groups

This file defines group markings and induces a norm on marked groups.
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there a reference you can give here? I'm not familiar with this definition

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I can't find a reference, but eg this MathOverflow question says "A $k$-marked groups is a pair $(G,S)$ where $G$ is a group and $S$ is an ordered set of $k$ generators of $G$." which you can see is the same definition as I have here, only that the set of generators can be extended to give a hom from the free group.

Copy link
Contributor

Choose a reason for hiding this comment

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

If there isn't a reference for a definition you wish to add, does it really belong in mathlib?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Okay, here's a reference: https://math.huji.ac.il/~perin/Documents/GGT/GGTCourse.pdf, Remark 5.3

Copy link
Contributor

Choose a reason for hiding this comment

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

Please add this to the module docstring. It would also be nice to also add the explanation of why you choose a definition different from the one which seems more standard.

@b-mehta b-mehta added the awaiting-author A reviewer has asked the author a question or requested changes. label Dec 17, 2024
@leanprover-community-bot-assistant leanprover-community-bot-assistant added the merge-conflict The PR has a merge conflict with master, and needs manual merging. (this label is managed by a bot) label Jan 31, 2025
@YaelDillies YaelDillies removed awaiting-author A reviewer has asked the author a question or requested changes. merge-conflict The PR has a merge conflict with master, and needs manual merging. (this label is managed by a bot) labels Jan 31, 2025
Comment on lines 18 to 20
* `MarkedGroup`: If `m : GroupMarking G S`, then `MarkedGroup m` is a type synonym for `G`
endowed with the metric coming from `m`.
* `MarkedGroup.instNormedGroup`: A marked group is normed by its marking.
Copy link
Contributor

Choose a reason for hiding this comment

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

It would be nice to give here a clearer description of what the metric and norm actually are. Currently the documentation only says that a norm is induced, with no clue as to how they are defined, or what they mean.

@[to_additive]
lemma norm_def [DecidableEq S] (x : MarkedGroup m)
[DecidablePred fun n ↦ ∃ l, toMarkedGroup (m l) = x ∧ l.toWord.length = n] :
‖x‖ = Nat.find (mul_aux' x) := by
Copy link
Contributor

Choose a reason for hiding this comment

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

It's generally not a great idea to have a lemma of this form, where the definition depends on a private lemma. You should instead give properties of the norm which can be useful without referencing private lemmas. For instance, the lemma that there exists a word in the free group with length n which corresponds to x is tricky to prove from the public API. This, and related lemmas (the norm is the smallest n with that property) should be included as part of the norm API.

@b-mehta b-mehta added the awaiting-author A reviewer has asked the author a question or requested changes. label Jan 31, 2025
@YaelDillies
Copy link
Collaborator Author

I've realised that I'm basically redoing the content of Analysis.Normed.Group.Quotient for the multiplicative non-abelian group FreeGroup S, so I've opened #21341 and #21342.

@YaelDillies YaelDillies added WIP Work in progress and removed awaiting-author A reviewer has asked the author a question or requested changes. labels Feb 2, 2025
@mathlib4-dependent-issues-bot mathlib4-dependent-issues-bot added the blocked-by-other-PR This PR depends on another PR (this label is automatically managed by a bot) label Feb 2, 2025
@mathlib4-dependent-issues-bot
Copy link
Collaborator

This PR/issue depends on:

@github-actions github-actions bot added the large-import Automatically added label for PRs with a significant increase in transitive imports label Feb 3, 2025
@leanprover-community-bot-assistant leanprover-community-bot-assistant added the merge-conflict The PR has a merge conflict with master, and needs manual merging. (this label is managed by a bot) label Feb 16, 2025
@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 Feb 16, 2025
@leanprover-community-bot-assistant leanprover-community-bot-assistant 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 17, 2025
@github-actions github-actions bot removed merge-conflict The PR has a merge conflict with master, and needs manual merging. (this label is managed by a bot) large-import Automatically added label for PRs with a significant increase in transitive imports labels Mar 18, 2025
@leanprover-community-bot-assistant leanprover-community-bot-assistant added the merge-conflict The PR has a merge conflict with master, and needs manual merging. (this label is managed by a bot) label Apr 11, 2025
@leanprover-community-bot-assistant
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 Apr 11, 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 19, 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 19, 2025
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-algebra Algebra (groups, rings, fields, etc) WIP Work in progress

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants