Skip to content

feat(RingTheory/MvPowerSeries/NoZeroDivisors): simplify the proof by adding two instances#36892

Open
AntoineChambert-Loir wants to merge 10 commits intoleanprover-community:masterfrom
AntoineChambert-Loir:ACL-EWInstances
Open

feat(RingTheory/MvPowerSeries/NoZeroDivisors): simplify the proof by adding two instances#36892
AntoineChambert-Loir wants to merge 10 commits intoleanprover-community:masterfrom
AntoineChambert-Loir:ACL-EWInstances

Conversation

@AntoineChambert-Loir
Copy link
Copy Markdown
Collaborator

@AntoineChambert-Loir AntoineChambert-Loir commented Mar 20, 2026

Clean up unwanted declarations regarding to well-orderings.

  • add exists_wellFoundedGTthat gives a linear ordering that satisfies WellFoundedGT
  • rename exists_wellOrder to exists_wellFoundedLT, deprecate the previous name, and add to_dual existing to relate the two functions.
  • add the instanceWellFoundedLT.wellOrderingRel suggested by @eric-wieser : WellOrderingRel.isWellOrder satisfies WellFoundedLT
  • adjust at a handful places, in particular in the proof of MvPowerSeries.NoZeroDivisors that the power series rings over a ring without zero divisors has no zero divisors, where the opposite of a “anti-well ordering” was needed.
  • delete LinearOrder.swap which abuses defeq (and was only used once for that proof).

Ref. #mathlib4 > help in RingTheory.MvPowerSeries.NeZeroDivisors


Open in Gitpod

@github-actions
Copy link
Copy Markdown

github-actions bot commented Mar 20, 2026

PR summary b43655dfe2

Import changes for modified files

No significant changes to the import graph

Import changes for all files
Files Import difference

Declarations diff

+ exists_wellFoundedGT
+ exists_wellFoundedLT

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

@vihdzp
Copy link
Copy Markdown
Collaborator

vihdzp commented Mar 20, 2026

Can we add to_dual to both of these?

@JovanGerb
Copy link
Copy Markdown
Contributor

JovanGerb commented Mar 20, 2026

Note that the dual instance gets different discrimination tree keys. The whole situation seems a bit dodgy

instance WellFoundedGT.swap {l : LinearOrder σ} [WellFoundedLT σ] :
    letI : LinearOrder σ := l.swap _; WellFoundedGT σ :=
  ‹WellFoundedLT σ›

instance WellFoundedLT.swap {l : LinearOrder σ} [WellFoundedGT σ] :
    letI : LinearOrder σ := l.swap _; WellFoundedLT σ :=
  ‹WellFoundedGT σ›

#discr_tree_key WellFoundedGT.swap -- IsWellFounded _ <other>
#discr_tree_key WellFoundedLT.swap -- IsWellFounded _ (@LT.lt _ _)

Recall that <other> means that it matches with any lambda expression.

@mariainesdff
Copy link
Copy Markdown
Contributor

@eric-wieser, could you please have a look? This comes from the discussion in #mathlib4 > help in RingTheory.MvPowerSeries.NeZeroDivisors; it looks ok to me but I might be missing something.

@JovanGerb
Copy link
Copy Markdown
Contributor

In my opinion, LinearOrder.swap should not exist in the first place. Could you instead refactor that proof to not use LinearOrder.swap, and then deprecate LinearOrder.swap? It is the only use of LinearOrder.swap

@eric-wieser
Copy link
Copy Markdown
Member

Sorry for dropping the ball here; I had not thought to look at the discrimination trees, and indeed these seem like a mark against this strategy. I'll investigate a little more now.

@JovanGerb
Copy link
Copy Markdown
Contributor

Note also that there does not exist anything similar to LinearOrder.swap for any other type class.

@AntoineChambert-Loir
Copy link
Copy Markdown
Collaborator Author

I will try to do as @JovanGerb requested. On the other hand, I would like to have a better understanding of the discussion — could you give me pointers to this question of discrimination trees, I'm ashamed to admit that I can't even parse Lean's output in Jovan's message.


variable (α) in
/-- The **well-ordering theorem** (or **Zermelo's theorem**): every type has a well-order -/
theorem exists_wellOrder : ∃ (_ : LinearOrder α), WellFoundedLT α := by
Copy link
Copy Markdown
Collaborator

@vihdzp vihdzp Apr 10, 2026

Choose a reason for hiding this comment

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

Perhaps this should be renamed exists_wellFoundedLT? And perhaps we can generate the other one via to_dual? (or at least tag it with to_dual existing)

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

About the renaming, I'm OK for that, it's only used four or five times. But WellFoundedLT doesn't impose LinearOrder, so I'm not sure that the name will truly reflect what is done. (And the remark applies to the new version exists_wellFoundedLT). Give a firm order and I'll do it.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

I do not understand the remark about to_dual, because at that point, one has no LinearOrder on the type.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

The previous name is even worse imo since we don't have anything called WellOrder (save for an auxiliary structure in the definition of ordinals, which this is not about). If you think this should reference linearOrder then why not exists_linearOrder_wellFoundedLT? Though this feels a bit verbose and I still prefer exists_wellFoundedLT.

Maybe this change can be split out of the PR? Seems like the stuff with LinearOrder.swap is a bit controversial, and the new WellFoundedGT theorem seems like a good, orthogonal change.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I do not understand the remark about to_dual, because at that point, one has no LinearOrder on the type.

Does to_dual existing not work regardless?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Maybe @AntoineChambert-Loir is not familiar with the @[to_dual] attribute for generating dual versions of declarations?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Indeed, I hadn't understood that it was similar to @[to_additive], I confused it with something applied to OrderDual.
Now, to_dual doesn't work, but to_dual existing worked. I also renamed the thing.

@JovanGerb
Copy link
Copy Markdown
Contributor

As far as I know, there isn't really a comprehensive writeup of what discrimination trees do in Lean. I should maybe write some :).

In short a discrimination tree is a data structure for storing and looking up expression patterns. The is used in simp and in type class search in order to get a list of candidate simp lemmas or candidate instances. Each simp lemma/instance is stored in the discrimination tree using a list of discrimination tree keys. It's helpful to know what the keys for a specific lemma/instance are, because if the keys are wrong/too specific, it may not get found. And on the other hand if the keys are too general, then it will be found too often, which is bad for performance.

@JovanGerb
Copy link
Copy Markdown
Contributor

But I think the discrimination tree keys aren't the main reason why I didn't like the instance. Rather, I had a problem with the swapping of the order in the funny LinearOrder instance. Instead, using OrderDual would be the right approach for that.

@AntoineChambert-Loir
Copy link
Copy Markdown
Collaborator Author

I don't see how to use OrderDual for that. You can impose an well ordering (wellFoundedLT) on OrderDual α, but that won't give you anything on α itself…

Comment on lines +143 to +144
let : LinearOrder σ := (exists_wellFoundedGT σ).choose
have : WellFoundedGT σ := (exists_wellFoundedGT σ).choose_spec
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Rather than using choose and choose_spec, you should be able to use rcases, right?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Yes. Is it better?

@JovanGerb
Copy link
Copy Markdown
Contributor

but that won't give you anything on α itself…

Well, OrderDual α is definitionally equal to α, so if you are happy abusing this defeq, then you can do it. This trick is used widely in Mathlib, though there are now people saying that this kind of defeq abuse is actually a bad idea.

@AntoineChambert-Loir
Copy link
Copy Markdown
Collaborator Author

Wouldn't it be tantamount to the proof to which the first round of review objected!

@AntoineChambert-Loir
Copy link
Copy Markdown
Collaborator Author

This PR is only 16 lines, I rewrote the description and it looks fairly coherent to me. May I beg you not to ask me to split into two or three tiny parts?

⟨⟨WellOrderingRel, inferInstance⟩⟩

/-- This instance is about `WellOrderingRel.isWellOrder.linearOrder`. -/
instance WellFoundedLT.wellOrderingRel :
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Do we need this instance in this PR? I would be against adding it.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

This is the other one suggested by Eric. The (initial) point was to remove the defeq abuses related to LinearOrder.swap, maybe that one is no more necessary.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

I'm commenting it out, and if that compiles, which it should, I'll remove it.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

(This PR started doing something, and made something else, probably cleaner, maybe that's the joy of collaborative work!)

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

Labels

t-ring-theory Ring theory

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants