search

Zolkin1 / torc

TORC: Tools for Optimization, Robotics, and Control
1 stars 1 forks source link

Implementation of quadratic costs and tests #5

Closed gavin-hyl closed 6 months ago

gavin-hyl commented 6 months ago

The current implementation's constructor takes in a matrix, but the different cases deserve a bit of clarification. If the user passes in an upper triangluar matrix Au, then we calculate the cost (and grad and Hessian) with f(x) = x^T Au x. However, if the matrix is not upper triangular, then the lower triangle (exclusing the main diagonal) is "folded" upward, essentially defining f(x) = x^T (Au + Al^T) x, where Au is the upper triangle matrix and Al is the stricly lower. Is this the behavior we want?

Zolkin1 commented 6 months ago

Also - when do a new push, hopefully it automatically runs all the tests here. I'll be monitoring that to make sure that the CI workflow I added works as expected.

Zolkin1 commented 6 months ago

Have you made the changes to template the class yet? I am actually thinking that we may want to keep everything dynamically sized. I have a feeling that fixing the sizes at compile time might cause problems down the road. It's hard for me to say for sure, but I think this will at least make it easier at first.

gavin-hyl commented 6 months ago

I have, but I was struggling with the test case implementations. I can change it back if you believe dynamically sized will be more convenient.

From: Zach Olkin @.> Sent: Thursday, May 2, 2024 20:55 To: Zolkin1/torc @.> Cc: Gavin Hua @.>; Author @.> Subject: Re: [Zolkin1/torc] Implementation of quadratic costs and tests (PR #5)

Have you made the changes to template the class yet? I am actually thinking that we may want to keep everything dynamically sized. I have a feeling that fixing the sizes at compile time might cause problems down the road. It's hard for me to say for sure, but I think this will at least make it easier at first.

— Reply to this email directly, view it on GitHubhttps://github.com/Zolkin1/torc/pull/5#issuecomment-2092076344, or unsubscribehttps://github.com/notifications/unsubscribe-auth/BBLXQMIJC64M6YRT5EWQZ7LZAMDETAVCNFSM6AAAAABGYJ5BQGVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDAOJSGA3TMMZUGQ. You are receiving this because you authored the thread.Message ID: @.***>

Zolkin1 commented 6 months ago

Yea, sorry to flip flop on you but let's change it back. Hopefully it's not too bad to do.

Zolkin1 commented 6 months ago

Really the one thing I'd like you to check on is the cmake Eigen stuff being in the write file. Besides that it looks good and I'll get it merged in when you make that change.