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(ring_theory/finiteness): generalize module.finite to noncommutative setting #9860

Closed
wants to merge 3 commits into from

Conversation

riccardobrasca
Copy link
Member

An almost for free generalization of module.finite to semiring.


Open in Gitpod

@riccardobrasca riccardobrasca added the awaiting-review The author would like community review of the PR label Oct 21, 2021
Comment on lines +84 to +85
lemma of_injective {R M N : Type*} [ring R] [add_comm_group M] [module R M] [add_comm_group N]
[module R N] [is_noetherian R N] (f : M →ₗ[R] N)
Copy link
Member

Choose a reason for hiding this comment

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

It would be straightforward to make this only require semiring R, as far as I can tell the proof just rewrites ker f = \bot back into function.injective f inside fg_of_injective

Copy link
Member Author

Choose a reason for hiding this comment

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

linear_map.ker_eq_bot.2 can be replaced by linear_map.ker_eq_bot_of_injective, that works for semiring, but fg_of_injective is for ring. Let me see if it easy to generalize.

Copy link
Member Author

Choose a reason for hiding this comment

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

Mmm, this is annoying: fg_of_injective does not have function.injective f as assumption, it has f.ker = ⊥, so I think it is true only over ring. But with function.injective f it would be true over semiring, and over rings f.ker = ⊥ implies function.injective f. I can change fg_of_injective, but it is better to do it in another PR.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm happy to leave that as a follow-up for when (if?) it is needed in the future.

Copy link
Member

Choose a reason for hiding this comment

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

Yes, the other half of my suggestion is to change fg_of_injective to assume function.injective f. I don't feel strongly about whether this is part of this PR, but I think it would be straightforward.

Copy link
Member Author

Choose a reason for hiding this comment

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

I agree it's straightforward, but then a modification will be needed everywhere it is used (a trivial one of course, but maybe in a lot of places). I am trying. If it is in a couple of files I will do it in this PR.

Copy link
Member Author

@riccardobrasca riccardobrasca Oct 22, 2021

Choose a reason for hiding this comment

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

It's not reasonable to do it here. ring_theory/noetherian can be generalized to semiring at lot, but even if almost all results trivially generalize, there is some work to do for others. I would say this is a job, if needed, for another PR.

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.

LGTM. I'll let @eric-wieser have the final decision whether the changes to noetherian.lean should be included.

@riccardobrasca
Copy link
Member Author

I am opening another PR, depending on this one, to fix the problem.

@eric-wieser
Copy link
Member

bors merge

Thanks!

@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 Oct 22, 2021
bors bot pushed a commit that referenced this pull request Oct 22, 2021
…ive setting (#9860)

An almost for free generalization of `module.finite` to `semiring`.
@bryangingechen bryangingechen changed the title feat(ring_theory/finiteness): generalize module.finite to noncommutative setting [Merged by Bors] - feat(ring_theory/finiteness): generalize module.finite to noncommutative setting Oct 22, 2021
@bryangingechen
Copy link
Collaborator

This was merged, but bors crashed.

@bors bors bot deleted the RB_generalize_finite branch October 22, 2021 18:56
bors bot pushed a commit that referenced this pull request Oct 23, 2021
We generalize some of the results in `ring_theory/noetherian` to `semiring`.

- [x] depends on: #9860



Co-authored-by: Johan Commelin <johan@commelin.net>
bors bot pushed a commit that referenced this pull request Oct 25, 2021
…l using module.finite (#9854)

`finite_dimensional K V` is by definition `is_noetherian K V`. We refactor this to use everywhere `finite K V` instead.

To keep the PR reasonably small, we don't delete `finite_dimension`, but we define it as `module.finite`. In a future PR we will remove it.

- [x] depends on: #9860
@eric-wieser eric-wieser added the hacktoberfest-accepted Without this label hacktoberfest is scared off by bors label Oct 26, 2021
ericrbg pushed a commit that referenced this pull request Nov 9, 2021
…ive setting (#9860)

An almost for free generalization of `module.finite` to `semiring`.
ericrbg pushed a commit that referenced this pull request Nov 9, 2021
We generalize some of the results in `ring_theory/noetherian` to `semiring`.

- [x] depends on: #9860



Co-authored-by: Johan Commelin <johan@commelin.net>
ericrbg pushed a commit that referenced this pull request Nov 9, 2021
…l using module.finite (#9854)

`finite_dimensional K V` is by definition `is_noetherian K V`. We refactor this to use everywhere `finite K V` instead.

To keep the PR reasonably small, we don't delete `finite_dimension`, but we define it as `module.finite`. In a future PR we will remove it.

- [x] depends on: #9860
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
hacktoberfest-accepted Without this label hacktoberfest is scared off by bors 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

4 participants