Skip to content
This repository has been archived by the owner on Jul 24, 2024. It is now read-only.

[Merged by Bors] - feat(ring_theory/derivations): stab on derivations #3688

Closed
wants to merge 23 commits into from

Conversation

Nicknamen
Copy link
Collaborator

@Nicknamen Nicknamen commented Aug 4, 2020


Temporary stab on derivations. I will write myself the definitive definition as soon as bimodules will be there.

@Vierkantor Vierkantor self-assigned this Aug 5, 2020
Copy link
Collaborator

@Vierkantor Vierkantor left a comment

Choose a reason for hiding this comment

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

Thank you for the PR!

I would have preferred it if you submitted three different PRs, one for cancel_monoid, one for compat_semimodule and one for derivation. Together they are still small enough, so I can review them.

src/algebra/group/basic.lean Outdated Show resolved Hide resolved
src/algebra/group/basic.lean Outdated Show resolved Hide resolved
src/algebra/group/basic.lean Outdated Show resolved Hide resolved
src/algebra/group/basic.lean Outdated Show resolved Hide resolved
src/ring_theory/derivations.lean Outdated Show resolved Hide resolved
src/ring_theory/derivations.lean Outdated Show resolved Hide resolved
src/ring_theory/derivations.lean Outdated Show resolved Hide resolved
src/ring_theory/derivations.lean Outdated Show resolved Hide resolved
src/ring_theory/derivations.lean Outdated Show resolved Hide resolved
src/ring_theory/derivations.lean Outdated Show resolved Hide resolved
@Vierkantor Vierkantor added the awaiting-author A reviewer has asked the author a question or requested changes label Aug 5, 2020
@Vierkantor
Copy link
Collaborator

Sorry for noticing it later on: could you move ring_theory/derivations.lean to ring_theory/derivation.lean? Files generally don't use a plural.

@ocfnash
Copy link
Collaborator

ocfnash commented Aug 7, 2020

I'm delighted to see this work taking shape but I wonder if bundling derivations into a structure is the best approach. I came here to comment that, mathematically, what is really being proved in derivation.commutator is that the derivations form a Lie subalgebra of the linear endomorphisms of the algebra A. But when I tried to drop in a proof of this I realised that because derivations are bundled I couldn't even state this fact!

Unfortunately I am running late for an appointment so I haven't time to finish this but here's another approach that might be worth considering:

-- Test this out by dropping it in at the end of your final section.

def is_derivation (D : A →ₗ[R] M) := ∀ ⦃a b⦄, D (a * b) = (a • D b) + (b • D a)

lemma is_derivation.eq (D : A →ₗ[R] M) (h : is_derivation D) :
  ∀ {a b}, D (a * b) = (a • D b) + (b • D a) := h

def submodule : submodule R (module.End R A) :=
{ carrier   := is_derivation,
  zero_mem' := λ a b, by simp,
  add_mem'  := λ a b ha hb, by sorry,
  smul_mem' := λ c a ha, by sorry, }

def lie_subalgebra : lie_subalgebra R (module.End R A) :=
{ lie_mem := λ D₁ D₂ h₁ h₂ a b, by { -- Awful proof but haven't time to fix.
    simp only [lie_ring.of_associative_ring_bracket,
      id.smul_eq_mul, linear_map.mul_app, linear_map.sub_apply],
    rw [h₁, h₂],
    simp only [id.smul_eq_mul, linear_map.map_add],
    rw [h₁, h₂, h₁, h₂],
    simp only [id.smul_eq_mul],
    ring, },
  ..submodule }

You should have a bit less work to do this way because module and Lie algebra structures will be automatically induced and I expect it will be very useful later to know not just that the derivations form a module / Lie algebra but actually submodule / Lie subalgebra.

@Nicknamen
Copy link
Collaborator Author

Nicknamen commented Aug 8, 2020

Hi Oliver, thanks for commenting! I am perfectly aware of this problem (I was also when I did define derivations) and it actually arose before in the topology and geometry sections of the library with continuous and smooth bundled maps, where it is not possible to treat eg continuous maps valued in a group as a subgroup of all maps (see #3446). However, in order to define the Lie algebra of a Lie group I defined vector fields as derivations over the algebra of smooth maps valued in the field (next PR), and for consistency in that section of the library and for it to properly work the only possible implementation of vector fields is as structures (regardless of how the structure is defined I believe it needs to be a structure: is_vector_field would not fit very well in my opinion). This having been said I totally see your point and I agree that for the purpose itself of the algebra section of the library this is not a good way to implement it. What we can do, if you agree it could be a good idea, is to have both implementations existing and working, as it happens in many other parts of the library (such as linear maps, continuous maps, smooth maps). If you prefer, I can write in the introductory part of the documentation to implement is_derivation as TODO and encourage to use is_derivation instead of derivation for the algebra section. Note that one year ago, Kenny Lau also implemented derivations as a structure so I imagine he possibly also had in mind a double implementation like this.

Maybe there is a way to do state things for bundled derivations in the algebra section as well though (https://leanprover.zulipchat.com/login/#narrow/stream/113488-general/topic/Making.20a.20type.20a.20.22subtype.22.20of.20another might be relevant for the case of continuous functions: could we do something in the same style?)

@Nicknamen Nicknamen changed the title feat(ring_theory/derivations): stub on derivations. feat(ring_theory/derivations): stab on derivations. Aug 8, 2020
@Nicknamen Nicknamen changed the title feat(ring_theory/derivations): stab on derivations. feat(ring_theory/derivations): stab on derivations Aug 8, 2020
@ocfnash
Copy link
Collaborator

ocfnash commented Aug 8, 2020

Thanks for such a comprehensive reply @Nicknamen I now understand that you had already carefully considered the bundled vs. unbundled approach.

I like the approach of defining the bracket on vector fields by identifying them with the derivations as a Lie subalgebra of the linear endomorphisms of the smooth functions (I guess this uses Hadamard's Lemma?). I am a little surprised that you find an unbundled approach to derivations would not work well there but I confess I have not seen that work or thought much about it.

It may well be that we need both the bundled and unbundled approaches (together with the machinery to plumb these together). If this turns out to be the case then I would advocate defining the module and Lie algebra structures on derivations via their submodule and Lie subalgebra structures using the unbundled definitions. Still, I appreciate I'm not in the weeds here so I am more than happy to leave this up to you.

I mostly just wanted to make sure these ideas were being considered and I now know they are. Thanks again for doing all this excellent work.

@Nicknamen
Copy link
Collaborator Author

Ok I believe this is pretty much ready on my side. I replaced compatible_semimodule with is_scalar_tower.

@Nicknamen Nicknamen added awaiting-review The author would like community review of the PR and removed awaiting-author A reviewer has asked the author a question or requested changes labels Aug 12, 2020
@github-actions github-actions bot removed the merge-conflict Please `git merge origin/master` then a bot will remove this label. label Aug 16, 2020
@Nicknamen
Copy link
Collaborator Author

Nicknamen commented Aug 16, 2020

The last commit I pushed will not compile but is there just to illustrate a refactor I could include with this PR. Everything there compiles up to the file ring_theory.algebra. Some of the lemmas that were proved for restrict_scalars are true for the general case of a scalar tower. What I did was to write an instance of is_scalar_tower for the restrict_scalars case and delete some lemmas of restrict_scalars to use only the is_scalar_tower version. However now Lean cannot resolve type class inference well as before, and in order to find the correct instances one now needs to specify the arguments of lemmas. It will not anymore be sufficient to write rw lemma_name but one will have to write rw lemma_name a b c. Should I go on with this refactor? Do we want this or is it better to keep both versions of lemmas there?

@Nicknamen
Copy link
Collaborator Author

Also, there are three pending changes that I did not apply because I would like to discuss them further!

@Nicknamen
Copy link
Collaborator Author

Nicknamen commented Aug 16, 2020

The last commit I pushed will not compile but is there just to illustrate a refactor I could include with this PR. Everything there compiles up to the file ring_theory.algebra. Some of the lemmas that were proved for restrict_scalars are true for the general case of a scalar tower. What I did was to write an instance of is_scalar_tower for the restrict_scalars case and delete some lemmas of restrict_scalars to use only the is_scalar_tower version. However now Lean cannot resolve type class inference well as before, and in order to find the correct instances one now needs to specify the arguments of lemmas. It will not anymore be sufficient to write rw lemma_name but one will have to write rw lemma_name a b c. Should I go on with this refactor? Do we want this or is it better to keep both versions of lemmas there?

Well this seems to happen just for the lemma semimodule.restrict_scalars_smul_def. I believe a good compromise could be to include this lemma only. In any case I believe the best thing is to remove semimodule.restrict_scalars from mathlib and substitute it with is_scalar_tower: in abstract lemmas it should be no problem to require these two additional instances and in practical examples, it should be always possible to define such instances. Should I propose this in Zulip?

@Vierkantor
Copy link
Collaborator

Refactoring restrict_scalars (i.e. deleting them) sounds like a good idea to me! Let's make a new PR for that and focus on derivations in this one, so we can merge it soon. I have some time now to do a review if you are ready for it.

@Nicknamen
Copy link
Collaborator Author

A review of derivations? Yeah sure I'd merge this first so that I can go on with the other PRs I have in cue! I am not sure it is best to review this before mathlib build succeeds, but if you don't mind go ahead! In any case we could start by resolving the three points from the previous review that are still open!

@Nicknamen
Copy link
Collaborator Author

Ok mathlib built so feel free to go on with the next review!

Copy link
Collaborator

@Vierkantor Vierkantor left a comment

Choose a reason for hiding this comment

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

Just one extra comment, apart from those comments everything looks good to me!

src/ring_theory/derivation.lean Outdated Show resolved Hide resolved
@Nicknamen Nicknamen added awaiting-review The author would like community review of the PR and removed awaiting-author A reviewer has asked the author a question or requested changes labels Aug 16, 2020
Copy link
Collaborator

@Vierkantor Vierkantor 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 fixes!

bors r+

@github-actions github-actions bot added ready-to-merge All that is left is for bors to build and merge this PR. (Remember you need to say `bors r+`.) and removed awaiting-review The author would like community review of the PR labels Aug 16, 2020
bors bot pushed a commit that referenced this pull request Aug 16, 2020
@bors
Copy link

bors bot commented Aug 16, 2020

Pull request successfully merged into master.

Build succeeded:

@bors bors bot changed the title feat(ring_theory/derivations): stab on derivations [Merged by Bors] - feat(ring_theory/derivations): stab on derivations Aug 16, 2020
@bors bors bot closed this Aug 16, 2020
@bors bors bot deleted the derivations branch August 16, 2020 22:17
bors bot pushed a commit that referenced this pull request Nov 19, 2020
This means that `group.to_left_cancel_semigroup` is now spelt `group.to_cancel_monoid.to_left_cancel_monoid.to_left_cancel_semigroup`.
The longer spelling shouldn't actually matter because type inference will do it anyway.

I don't know whether this matters, but this should slightly reduce the number of connections that instance resolution must check.

This shortcut wasn't added deliberately, it seems it just got added accidentally when #3688 was introduced.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
ready-to-merge All that is left is for bors to build and merge this PR. (Remember you need to say `bors r+`.)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants