Quadratic Objective Function to support basic vector operations #858
Conversation
…of the quadratic objective function. - Switched the ConstantTerm to an auto-property (cleaner) - Moved parsing logic to its own static class, QuadraticExpressionParser.cs (cleaner). TODO recycle parsing logic in QuadraticConstraint?? - Fixing issue where Expressions are incorrectly parsed. For instance f = xy + yx fails because duplicate terms are only counted once. - Speeding up gradient calculation by removing weird transpose. *** MAJOR ISSUE: calculation is STILL incorrect for non-symmetric matrices ***. Need to decide the best fix. Either 0.5(Q+Q') or, simply, Q but force Q to be symmetric. Adding a check on whether the quadratic and linear dictionaries are null and returning an alternative string in these cases. This fixes GH-832. Not including zeros because f = 2x+0y+1z is confusingly shown as "+2x +y +1z!"
… quadratic and linear dictionaries.
# Conflicts: # Sources/Accord.Math/Matrix/Matrix.Common.cs # Unit Tests/Accord.Tests.Math/Optimization/QuadraticObjectiveFunctionTest.cs
…ing unit tests for non-symmetric matrices.
…into development
| { 1, 3, 1 }, | ||
| { 3, 2, 2 }, | ||
| { 1, 2, 3 }, | ||
| { 8, 3, 1 }, |
There was a problem hiding this comment.
I guess I am ok with the change, but why was it needed?
There was a problem hiding this comment.
Ah I see :D I must admit I spent more than 20 mins trying to remember why this was actually changed (it wasn't really a deliberate change). Anyways, I remember now...
The ToString() method only displays the coefficients to the nearest integer (it's always been like that - not a change from me). As that test has odd numbers down the diagonal, the ToString() looked funny. I just switched it to even numbers to verify I hadn't broken it. I can revert it if you like but I presume(d) those numbers were arbitrary.
| @@ -242,8 +241,8 @@ public void FunctionTest() | |||
| double e = expected(); | |||
|
|
|||
| Assert.AreEqual(e, a, 1e-10); | |||
| Assert.IsFalse(Double.IsNaN(a)); | |||
| Assert.IsFalse(Double.IsNaN(e)); | |||
| Assert.IsFalse(double.IsNaN(a)); | |||
There was a problem hiding this comment.
By the way, just in case you might be wondering, the checks against NaN even though we already had checked for equality were needed when the project used MSTest for unit tests. Now that we use NUnit3, they shouldn't be that much necessary (but they also don't hurt anyway).
| @@ -127,20 +145,23 @@ namespace Accord.Math.Optimization | |||
| /// double value = solver.Value; | |||
| /// double[] solutions = solver.Solution; | |||
| /// </code> | |||
| /// | |||
| /// <para> | |||
| /// Sometimes it easiest to compose quadratic functions as linear | |||
There was a problem hiding this comment.
Is there a typo here? Sorry I am not a native English speaker, so I thought it was better to ask :-)
There was a problem hiding this comment.
There was indeed. Thanks for spotting. I have fixed the typo and pushed. BTW - if you want a native speaker to review your arXiv article I am more than happy to. However, to be completely clear, your English seems absolutely faultless - and I make typos :) - so I'm not sure I see the need!
| throw new DimensionMismatchException("quadraticTerms", "The matrix must be square."); | ||
|
|
||
| if (quadraticTerms.GetLength(0) != linearTerms.Length) | ||
| if (!quadraticTerms.IsSymmetric(rtol: 1e-6)) |
There was a problem hiding this comment.
I think this is fine. If for some reason it throws for a matrix that was indeed supposed to be symmetric, the user can always "force" it to be symmetric with GetSymmetric or manually.
There was a problem hiding this comment.
Cool - my thinking was similar and I had spotted that method - would work well. Another pair of methods that would be useful would be GetSymmetricPart() and GetAntiSymmetricPart evaluating 0.5(M+M') and 0.5(M-M') respectively.
| /// supports this by overloading the addition and multiplication operators. | ||
| /// </para> | ||
| /// | ||
| /// <code source="Unit Tests\Accord.Tests.Math\Optimization\QuadraticObjectiveFunctionTest.cs" region="doc_example" /> |
There was a problem hiding this comment.
It is perfect, thanks for following the style so closely!
|
Thanks a lot @AlexJCross! |
|
@cesarsouza as always, you're welcome. Thanks for accepting this PR! |
Hi @cesarsouza,
I know I can merge in directly but I'd really appreciate your thoughts on 3 points.
NonSymmetricMatrixExceptionif relative tolerance is >1e-6. Does that sound reasonable to you? I didn't want the requirement for the matrix to be symmetric to be too much of a burden but i can still envisage scenarios where this might throw. Any better suggestions 100% welcome. Alternatively, if it detects a non-symmetric matrix, it could always grab the non-symmetric part and continue.IDictionary<string, int> Variables, their code would now break. It's also not consistent with theNonlinearObjectiveFunction. So I can: leave it as is; change the Nonlinear as well for consistency; revert completely; or simply change theToString()method aesthetically but not touch the underlying variable names.Reminder:
Happy to make changes as you see fit.
Thanks!
Alex
ps - this closes #819