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(algebra/big_operators/basic): add lemma finset.prod_dvd_prod #11521

Closed
wants to merge 6 commits into from

Conversation

stuart-presnell
Copy link
Collaborator

For any S : finset α, if ∀ a ∈ S, g1 a ∣ g2 a then S.prod g1 ∣ S.prod g2.


Open in Gitpod

@stuart-presnell stuart-presnell added the awaiting-review The author would like community review of the PR label Jan 17, 2022
lemma prod_dvd {S : finset α} (g1 g2 : α → β) (h : ∀ a ∈ S, g1 a ∣ g2 a) : S.prod g1 ∣ S.prod g2 :=
begin
classical,
apply finset.induction_on' S, { simp },
Copy link
Member

@eric-wieser eric-wieser Jan 18, 2022

Choose a reason for hiding this comment

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

Suggested change
apply finset.induction_on' S, { simp },
induction S using finset.induction_on' with a T haS _ haT IH,
{ simp },

Copy link
Member

Choose a reason for hiding this comment

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

Although I think you might not need the primed version here based on the _ in your intros below.

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 get this to work with the unprimed version, although the proof then becomes a bit longer:

lemma prod_dvd_prod {S : finset α} (g1 g2 : α → β) (h : ∀ a ∈ S, g1 a ∣ g2 a) :
  S.prod g1 ∣ S.prod g2 :=
begin
  classical,
  induction S using finset.induction_on with a T haT h1, { simp },
  repeat {rw finset.prod_insert haT},
  apply mul_dvd_mul,
  { apply h, simp },
  { refine h1 (λ x hx, _), simp [h, mem_insert_of_mem hx] },
end

However, if I try using the primed version as you first suggested I immediately and consistently get Server has stopped due to signal SIGSEGV as soon as the compiler hits that line. Any idea what's going wrong here, or should I just ask on Zulip?

Copy link
Member

Choose a reason for hiding this comment

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

If you reliably get a sigsegv, then I'd push the broken version and ask on zulip what's going on.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Hmm, just as mysteriously I'm no longer getting the SIGSEGV at all and I can't reproduce it.

Now induction S using finset.induction_on' just gives induction tactic failed, failed to create new goal (which is also strange, given that apply finset.induction_on' S does work).

Copy link
Member

Choose a reason for hiding this comment

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

Well, the style guide wants

Suggested change
apply finset.induction_on' S, { simp },
apply finset.induction_on' S,
{ simp },

Which means my spelling is fewer lines that the version matching the style

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Really? I understood the style guide to say that it's ok to immediately close a trivial goal on the same line, and it explicitly gives the example of induction n, { simp }. A quick search turns up lots of examples of induction ..., { simp... on a single line (and several with apply or refine).

Copy link
Collaborator Author

@stuart-presnell stuart-presnell Jan 23, 2022

Choose a reason for hiding this comment

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

And although your version is fewer lines, I can't find a way to get it to work in fewer characters since, as I noted above, induction S using finset.induction_on' doesn't seem to work for some reason, while induction S using finset.induction_on produces a longer proof.

Copy link
Member

Choose a reason for hiding this comment

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

I think we've always taken a pretty pragmatic approach to that aspect of the style guide.
Arguable the induction .. with .. shares more semantic information than the generic apply. But I wouldn't get hung up over this.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Ok, thanks. So is the proof ok as it stands (in this respect, at least)?

@jcommelin jcommelin 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 Jan 18, 2022
@stuart-presnell stuart-presnell 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 Jan 18, 2022
@stuart-presnell stuart-presnell changed the title feat(algebra/big_operators/basic): add lemma finset.prod_dvd feat(algebra/big_operators/basic): add lemma finset.prod_dvd_prod Jan 18, 2022
@robertylewis
Copy link
Member

LGTM!

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 Jan 25, 2022
bors bot pushed a commit that referenced this pull request Jan 25, 2022
…11521)

For any `S : finset α`, if `∀ a ∈ S, g1 a ∣ g2 a` then `S.prod g1 ∣ S.prod g2`.
@bors
Copy link

bors bot commented Jan 25, 2022

Pull request successfully merged into master.

Build succeeded:

@bors bors bot changed the title feat(algebra/big_operators/basic): add lemma finset.prod_dvd_prod [Merged by Bors] - feat(algebra/big_operators/basic): add lemma finset.prod_dvd_prod Jan 25, 2022
@bors bors bot closed this Jan 25, 2022
@bors bors bot deleted the SP_finset_prod_dvd branch January 25, 2022 05:11
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

4 participants