This repository has been archived by the owner on Jul 24, 2024. It is now read-only.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
[Merged by Bors] - feat(tactic/linear_combination): allow
linear_combination with { exponent := n }
#15428[Merged by Bors] - feat(tactic/linear_combination): allow
linear_combination with { exponent := n }
#15428Changes from 3 commits
688cedb
23352d9
bd90404
0cb3798
6f18ae9
101bcbd
9d86782
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This results in the slightly unhelpful help text,
I think either:
exponent
should appear in this help textexponent
a field oflinear_combination_config
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd lean towards the second;
linear_combination _ with { exponent := 2 }
is much easier to extend later, doesn't involve any surprising new syntax, and has default values built in.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not opposed, but there's another
linear_combination
feature under discussion that wouldn't fit this pattern. We're scheming up syntax for a "contraposed" version, something likelinear_combination 3*⊢ with target zero_ne_one
that would work on disequality goals and a single disequality hypothesis/proof term. I don't think we can take an arbitrary proof of an arbitrary proposition in the config structure with convenient syntax and a default value.So that would leave us at
linear_combination _ with target zero_ne_one {exponent := 2}
which is a little funny.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What would your proposed spelling me for both options simultaneously?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
At a glance, "linear_combination _ with target x" feels a bit like
simpa _ using x
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@eric-wieser I've taken your suggestion and reimplemented this as a field of the config object!