-
Notifications
You must be signed in to change notification settings - Fork 7
the L2-norm ball in case of a mixed-integer problem #269
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
base: main
Are you sure you want to change the base?
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #269 +/- ##
==========================================
- Coverage 86.23% 84.83% -1.40%
==========================================
Files 17 22 +5
Lines 1482 2691 +1209
==========================================
+ Hits 1278 2283 +1005
- Misses 204 408 +204 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
only 2normball here |
src/polytope_blmos.jl
Outdated
| count = 0 | ||
| for idx in 1:length(int_vars) | ||
| if !(lb[idx] <= 0 <= ub[idx]) | ||
| count = count + 1 |
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.
count += count
| if !(lb[idx] <= 0 <= ub[idx]) | ||
| count = count + 1 | ||
| end | ||
| if count > 1 |
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.
A comment would be helpful here, the code is not very clear.
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.
added the comment
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.
So we are just tackling the case of radius=1? Then, this has to be more clear in the function definition.
src/polytope_blmos.jl
Outdated
| BLMO denotes the L2normBall, It is unit ball which means R = 1 | ||
| """ | ||
|
|
||
| struct L2normBallBLMO <: FrankWolfe.LinearMinimizationOracle end |
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 don't think we need an extra struct here.
src/polytope_blmos.jl
Outdated
|
|
||
| struct L2normBallBLMO <: FrankWolfe.LinearMinimizationOracle end | ||
|
|
||
| function bounded_compute_extreme_point(blmo::L2normBallBLMO, d, lb, ub, int_vars; kwargs...) |
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.
Write instead
function bounded_compute_extreme_point(blmo::FrankWolfe.LpNormBallLMO{T,2}, d, lb, ub, int_vars; kwargs...)
...
end
And the same for the other two functions here.
| end | ||
|
|
||
| # Test for L2normBallBLMO | ||
| @testset "L2normBall BLMO continuous" begin |
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.
You are not testing the continuous case here, though.
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.
Now it test the continuous case
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.
You still have num_int=5. So it is still an integer problem.
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.
But for the optimum, the integer entries (dimension) are all 0, the values are all in continuous entries.
test/LMO_test.jl
Outdated
| lower_bounds = fill(-2.0, num_int) | ||
| upper_bounds = fill(2.0, num_int) |
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.
Directly, fill with 1.0 and -1.0 directly.
| # Generate a solution inside the L2 ball (||x|| <= 1) | ||
| x_sol = randn(rng, n) | ||
| x_sol = x_sol ./ (norm(x_sol) * 1.5) | ||
|
|
||
| # Round some coordinates to integers for testing | ||
| num_int = 5 | ||
| int_indices = sort(rand(rng, 1:n, num_int)) | ||
| for idx in int_indices | ||
| x_sol[idx] = 0 | ||
| end |
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.
How can you guarantee that x_sol will stay inside the ball? If you cannot, the tests later will fail even if we find the correct solution.
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.
x_sol = x_sol ./ (norm(x_sol) * 1.5) here guarantee that x_sol will stay inside the ball
test/LMO_test.jl
Outdated
| @test isapprox(f(x), f(result[:raw_solution]), atol=1e-6, rtol=1e-3) | ||
| end | ||
|
|
||
| @testset "L2normBall BLMO integer 1" begin |
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.
There is not difference between the tests? One is sufficient.
| """ | ||
| 2normBallBLMO() | ||
| BLMO denotes the L2normBall, It is unit ball which means R = 1 | ||
| """ | ||
|
|
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.
Please add some documentation here.
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.
Yes we are just tackling the case of radius=1. I am still thinking and working on the general case.
| if !(lb[idx] <= 0 <= ub[idx]) | ||
| count = count + 1 | ||
| end | ||
| if count > 1 |
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.
So we are just tackling the case of radius=1? Then, this has to be more clear in the function definition.
| end | ||
|
|
||
| # Test for L2normBallBLMO | ||
| @testset "L2normBall BLMO continuous" begin |
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.
You still have num_int=5. So it is still an integer problem.
No description provided.