Skip to content

VectorQuadraticFunction constructor #2633

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

Closed
wants to merge 5 commits into from

Conversation

matbesancon
Copy link
Contributor

In the same vein as #2626

@blegat
Copy link
Member

blegat commented Feb 17, 2025

Why not keep the existing implementation ?

Copy link
Member

@joaquimg joaquimg left a comment

Choose a reason for hiding this comment

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

using -> import

matbesancon and others added 2 commits February 17, 2025 17:57
Co-authored-by: Joaquim <joaquimdgarcia@gmail.com>
Co-authored-by: Joaquim <joaquimdgarcia@gmail.com>
@matbesancon matbesancon requested a review from joaquimg February 17, 2025 16:57
@matbesancon
Copy link
Contributor Author

Why not keep the existing implementation ?

I find the loop version simpler instead of having to mapreduce multiple times over the same vector

Comment on lines +685 to +689
ret = VectorQuadraticFunction{T}(
VectorQuadraticTerm{T}[],
VectorAffineTerm{T}[],
T[],
)
Copy link
Member

Choose a reason for hiding this comment

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

I mentioned in the other PR also:
If this constructor is just for testing purposes and manually building constraints, I would be okay with the simple loop.
But, vectorize is used in bridges in potentially large problems.
Hence, we need, at least, some sizehint!s arounds

@odow
Copy link
Member

odow commented Feb 17, 2025

Closing in favor of #2636

@odow odow closed this Feb 17, 2025
@matbesancon matbesancon deleted the vqf-constructor branch February 18, 2025 07:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

4 participants