-
Notifications
You must be signed in to change notification settings - Fork 37
Add merge_id
and generalize exact_sum
for SPZ
#3905
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
Add merge_id
and generalize exact_sum
for SPZ
#3905
Conversation
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.
Would be better to have a separate PR for each part - at least scale
seems independent.
12bbd9d
to
ab551eb
Compare
ab551eb
to
baf22f7
Compare
scale
and merge_id
functions for SPZmerge_id
functions for SPZ
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 still need to read the tests, but here are some suggestions.
src/Sets/SparsePolynomialZonotope/SparsePolynomialZonotopeModule.jl
Outdated
Show resolved
Hide resolved
src/Sets/SparsePolynomialZonotope/SparsePolynomialZonotopeModule.jl
Outdated
Show resolved
Hide resolved
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.
Reviewed everything. LGTM after the suggested changes!
Co-authored-by: Christian Schilling <[email protected]>
Co-authored-by: Christian Schilling <[email protected]>
Co-authored-by: Christian Schilling <[email protected]>
Co-authored-by: Christian Schilling <[email protected]>
Co-authored-by: Christian Schilling <[email protected]>
Co-authored-by: Christian Schilling <[email protected]>
Co-authored-by: Christian Schilling <[email protected]>
Co-authored-by: Christian Schilling <[email protected]>
Co-authored-by: Christian Schilling <[email protected]>
Co-authored-by: Christian Schilling <[email protected]>
Co-authored-by: Christian Schilling <[email protected]>
I agree with all the suggestions, but one, see my comment above. |
src/Sets/SparsePolynomialZonotope/SparsePolynomialZonotopeModule.jl
Outdated
Show resolved
Hide resolved
Co-authored-by: Christian Schilling <[email protected]>
Co-authored-by: Christian Schilling <[email protected]>
Co-authored-by: Christian Schilling <[email protected]>
Co-authored-by: Christian Schilling <[email protected]>
merge_id
for SPZmerge_id
and generalize exact_sum
for SPZ
Update: I added the corrected
merge_id
function, including tests and documentation. I also revisitedexact_sum
function for SPZ to handle SPZs with differingindexVector
s using themerge_id
functions