Skip to content
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

Multi-threading on solving subproblems #345

Merged
merged 5 commits into from
May 26, 2020
Merged

Multi-threading on solving subproblems #345

merged 5 commits into from
May 26, 2020

Conversation

dianabarros
Copy link
Contributor

This implementation needs Gurobi to work.

@dianabarros
Copy link
Contributor Author

dianabarros commented May 20, 2020

@guimarqu @cristianabentes ,
this branch contains changes only related to the parallelization of the solve subproblems loop. For now, I’m using a global variable to say if we want to run the code using threads or not. I understand this might not be the best way of doing it, but I’d like to know what would be the best way of doing it for Coluna.
We could pass it as a parameter to the function but i don’t know at what point in the code we can set it.

I also didn't add Gurobi packages since a license is needed to run it, but it's important to note that this parallelization will only work with Gurobi. Should we keep it like this to merge with master?

@codecov
Copy link

codecov bot commented May 20, 2020

Codecov Report

Merging #345 into master will decrease coverage by 0.19%.
The diff coverage is 53.57%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #345      +/-   ##
==========================================
- Coverage   75.49%   75.29%   -0.20%     
==========================================
  Files          50       50              
  Lines        3664     3672       +8     
==========================================
- Hits         2766     2765       -1     
- Misses        898      907       +9     
Impacted Files Coverage Δ
src/Coluna.jl 100.00% <ø> (ø)
src/Algorithm/colgen.jl 81.20% <51.85%> (-2.53%) ⬇️
src/MathProg/optimizerwrappers.jl 70.44% <100.00%> (-0.19%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 91afce9...f005bb9. Read the comment docs.

src/Algorithm/colgen.jl Outdated Show resolved Hide resolved
@guimarqu
Copy link
Contributor

I also didn't add Gurobi packages since a license is needed to run it, but it's important to note that this parallelization will only work with Gurobi. Should we keep it like this to merge with master?

If I have my own thread-safe algorithm to solve the subproblem, it should work, isn't it ?
If yes, I'll write an algorithm to solve the subproblem and put that in tests (but no in this PR, I'll do it later).

@cristianabentes
Copy link
Contributor

cristianabentes commented May 21, 2020

If I have my own thread-safe algorithm to solve the subproblem, it should work, isn't it ?

Yes

If yes, I'll write an algorithm to solve the subproblem and put that in tests (but no in this PR, I'll do it later).

Ok, but meanwhile we need Gurobi. We will leave the execution with Gurobi only in our branch. On the master branch the parallelization will be turned off until the home-made solution is ready.

dual_sol::DualSolution, sp_lbs::Dict{FormId, Float64}, sp_ubs::Dict{FormId, Float64}
)
threadstasks = Task[]
Copy link
Contributor

Choose a reason for hiding this comment

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

It looks unused

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You're right, we don't need it anymore. I removed it.

@guimarqu
Copy link
Contributor

Close PR #341 #333 #217 #188

@guimarqu guimarqu merged commit 0ad2c75 into master May 26, 2020
@guimarqu guimarqu deleted the parallelGurobi3 branch May 26, 2020 07:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants