Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Merged by Bors] - feat(linear_algebra/tensor_algebra): Tensor algebras #3531

Closed
wants to merge 22 commits into from

Conversation

adamtopaz
Copy link
Collaborator

@adamtopaz adamtopaz commented Jul 23, 2020

This PR constructs the tensor algebra of a module over a commutative ring.

The main components are:

  1. The construction of the tensor algebra: tensor_algebra R M for a module M over a commutative ring R.
  2. The linear map univ R M from M to tensor_algebra R M.
  3. Given a linear map ffrom Mto an R-algebra A, the lift of f to tensor_algebra R M is denoted lift R M f.
  4. A theorem univ_comp_lift asserting that the composition of univ R M with lift R M fis f.
  5. A theorem lift_unique asserting the uniqueness of lift R M fwith respect to property 4.

Note: This PR only does the bare minimum!

@adamtopaz adamtopaz added the awaiting-review The author would like community review of the PR label Jul 23, 2020
Copy link
Member

@jcommelin jcommelin left a comment

Choose a reason for hiding this comment

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

Nice work!

src/linear_algebra/tensor_algebra.lean Outdated Show resolved Hide resolved
src/linear_algebra/tensor_algebra.lean Outdated Show resolved Hide resolved
src/linear_algebra/tensor_algebra.lean Outdated Show resolved Hide resolved
pre.has_coe_module pre.has_coe_ring pre.has_mul pre.has_add pre.has_zero
pre.has_one pre.has_scalar pre.has_neg

instance : ring (tensor_algebra R M) :=
Copy link
Member

Choose a reason for hiding this comment

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

This proof and the following one really seem to call for a small tactic, something like the pi_instance tactic.

@eric-wieser
Copy link
Member

eric-wieser commented Jul 24, 2020

Now that this is written in terms of [semiring R] and [semimodule M], is it still possible to show [ring (tensor_algebra R M)] if I have [ring R] [module M]?

@adamtopaz
Copy link
Collaborator Author

Now that this is written in terms of [semiring R] and [semimodule M], is it still possible to show [ring (tensor_algebra R M)] if I have [ring R] [module M]?

This is a more general question: if you have semiring which is an algebra over a ring, then it is also a ring.
Neg is defined as multiplication by -1 from the base ring, and distributivity should give you all the other lemmas.

But this proof probably doesn't belong in this PR.

Also, changed function ccomp in theorems to a comp of lin maps.
@eric-wieser
Copy link
Member

But this proof probably doesn't belong in this PR.

My fear is that such a proof might require an entirely different construction, perhaps as bad as having two entirely separate quot definitions

@semorrison
Copy link
Collaborator

But this proof probably doesn't belong in this PR.

My fear is that such a proof might require an entirely different construction, perhaps as bad as having two entirely separate quot definitions

I'm pretty confident this won't be the case. Given a semiring which is an algebra over a ring, it's straightforward to construct a ring instance.

@semorrison
Copy link
Collaborator

Besides some minor suggestions on doc-strings, let's hit merge!

@semorrison
Copy link
Collaborator

bors d+

@bors
Copy link

bors bot commented Jul 25, 2020

✌️ adamtopaz can now approve this pull request. To approve and merge a pull request, simply reply with bors r+. More detailed instructions are available here.

@semorrison semorrison added delegated The PR author may merge after reviewing final suggestions. and removed awaiting-review The author would like community review of the PR labels Jul 25, 2020
@semorrison
Copy link
Collaborator

Oops, removing that, another maintainer suggested we look more at this one.

bors d-

@semorrison semorrison added awaiting-author A reviewer has asked the author a question or requested changes and removed awaiting-review The author would like community review of the PR labels Jul 28, 2020
begin
refine ⟨λ hyp, _, λ hyp, by simp [hyp]⟩,
ext,
rcases quot.exists_rep x with ⟨x,rfl⟩,
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
rcases quot.exists_rep x with ⟨x,rfl⟩,
rcases x,

theorem lift_unique {A : Type u₃} [semiring A] [algebra R A] (f : M →ₗ[R] A)
(g : tensor_algebra R M →ₐ[R] A) : g.to_linear_map.comp (ι R M) = f ↔ g = lift R M f :=
begin
refine ⟨λ hyp, _, λ hyp, by simp [hyp]⟩,
Copy link
Member

Choose a reason for hiding this comment

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

Clearer without simp, maybe?

Suggested change
refine ⟨λ hyp, _, λ hyp, by simp [hyp]⟩,
refine ⟨λ hyp, _, λ hyp, by {rw hyp, exact ι_comp_lift f}⟩,


@[simp]
theorem ι_comp_lift {A : Type u₃} [semiring A] [algebra R A] (f : M →ₗ[R] A) :
(lift R M f).to_linear_map.comp (ι R M) = f := by {ext, refl}
Copy link
Member

@eric-wieser eric-wieser Jul 28, 2020

Choose a reason for hiding this comment

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

Is this similar theorem also useful?

@[simp]
theorem lift_comp_ι {A : Type u₃} [semiring A] [algebra R A] (g : tensor_algebra R M →ₐ[R] A) :
  lift R M (g.to_linear_map.comp (ι R M)) = g :=
begin
  symmetry,
  ext,
  rcases x,
  induction x,
  case pre.of: x { refl },
  case pre.of_scalar: x { exact alg_hom.commutes g x },
  case pre.add: a b ha hb { change g (quot.mk _ _ + quot.mk _ _) = _,
    rw alg_hom.map_add,
    rw [ha, hb], refl },
  case pre.mul: a b ha hb { change g (quot.mk _ _ * quot.mk _ _) = _,
    rw alg_hom.map_mul,
    rw [ha, hb], refl },
end

With it, you can implement lift_unique as ⟨λ hyp, by simp [←hyp], λ hyp, by simp [hyp]⟩

@adamtopaz
Copy link
Collaborator Author

Here is a summary of the changes.

  1. I removed the universe variables. As I mentioned in my response to the comment regarding this, the original proofs in lift broke, but after some small changes there, everything now works with Type*.
  2. I added implementation details in the documentation at the top of the file.
  3. I implemented the various changes that @semorrison suggested.
  4. I added the theorem that @eric-wieser suggested, except I proved it using the lift_unique theorem that was already there.
  5. Various other minor changes to the documentation and/or spacing.

Thanks @eric-wieser and @semorrison for the suggestions!

I want to reiterate that neither I nor @eric-wieser marked the conversations about the universe variables and pre as resolved. Since it was another maintainer that did this, I had the impression that the changes I made actually resolved these issues. You can take a look at the history and see that I only ever marked a conversation as resolved when I explicitly made the change that a maintainer has asked for.

I understand (and agree!) that this is a large project with a lot of moving pieces, and that style, formatting, and documentation is important. If there are some other issues, then please let me know what they are and I would be happy to change them. Other than that, I'll honestly say that I was a bit discouraged by the subtly hostile comments about "bad habits."

@PatrickMassot
Copy link
Member

I'm really sorry that you felt that my comment was "subtly hostile". This was not my intention at all. I don't understand why people marked these conversation as resolved, and I admit it didn't come to my mind that other maintainers could have done that. But all maintainers are under pressure from the huge flow of PRs. We are very happy to see all those contributions, but it's not always easy.

I'm a bit worried that changing the universes annotations required changes to the proofs. I think it's a very bad omen. We really need to understand what happened. The first step is to have a look at all the universe levels inferred by Lean before and after this change.

@adamtopaz
Copy link
Collaborator Author

adamtopaz commented Jul 28, 2020

Thanks @PatrickMassot -- no harm, no foul.

Concerning universes -- I just checked. The universe annotations work out to be the same. The three types R, M and A which appear in the file can be in arbitrary (and unrelated) universes. The only thing that changed in the proof is this:

Before:

repeat { set G := lift_fun R M f,
      change G _ + G _ = G _ + G _,
      rw h_ih, },
repeat { set G := lift_fun R M f,
      change G _ * G _ = G _ * G _,
      rw h_ih, },

After:

repeat { change lift_fun R M f _ + lift_fun R M f _ = _,
      rw h_ih,
      refl, },
repeat { change lift_fun R M f _ * lift_fun R M f _ = _,
      rw h_ih,
      refl, },

These seem to be equivalent to me! And certainly the second proof works even with complete universe annotations (I checked).
Finally, I should mention that the proof didn't really break down, it's just that some goals were not closed as part of the repeat. Long story short: I don't think there is any issue in using Type* everywhere.

@PatrickMassot
Copy link
Member

Ok, thanks for checking this. I'll let Scott check whether he's happy and merge.

@robertylewis robertylewis 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 Jul 29, 2020
@eric-wieser

This comment has been minimized.

@adamtopaz
Copy link
Collaborator Author

adamtopaz commented Jul 29, 2020

You need a comm_ring instance for R.

This is a restriction coming from the algebra typeclass

@eric-wieser
Copy link
Member

I'm being stupid, I misread comm_ring as semiring. Sorry for the noise.

@semorrison
Copy link
Collaborator

bors merge

@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 2, 2020
bors bot pushed a commit that referenced this pull request Aug 2, 2020
This PR constructs the tensor algebra of a module over a commutative ring.

The main components are:
1. The construction of the tensor algebra: `tensor_algebra R M` for a module `M` over a commutative ring `R`.
2. The linear map `univ R M` from `M` to `tensor_algebra R M`.
3. Given a linear map `f`from `M`to an `R`-algebra `A`, the lift of `f` to `tensor_algebra R M` is denoted `lift R M f`.
4. A theorem `univ_comp_lift` asserting that the composition of `univ R M` with `lift R M f`is `f`.
5. A theorem `lift_unique` asserting the uniqueness of `lift R M f`with respect to property 4.



Co-authored-by: Adam Topaz <adamtopaz@users.noreply.github.com>
Co-authored-by: Patrick Massot <patrickmassot@free.fr>
@bors
Copy link

bors bot commented Aug 2, 2020

Pull request successfully merged into master.

Build succeeded:

@bors bors bot changed the title feat(linear_algebra/tensor_algebra): Tensor algebras [Merged by Bors] - feat(linear_algebra/tensor_algebra): Tensor algebras Aug 2, 2020
@bors bors bot closed this Aug 2, 2020
@bors bors bot deleted the tensor_algebra branch August 2, 2020 13:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
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.

None yet

7 participants