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.

Sign up for GitHub

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

Fixed a bug when restoring master cuts in strong branching #444

Merged
merged 3 commits into from
Feb 11, 2021
Merged

Fixed a bug when restoring master cuts in strong branching #444

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
3ab7df6
Fixed a bug when restoring master cuts in strong branching. Added sev…
artalvpes Feb 6, 2021
2ae1f4f
Removing debug codes, stopping the branch evaluation on conquered bra…
artalvpes Feb 10, 2021
f250ab3
Correcting the style
artalvpes Feb 11, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion src/Algorithm/basic/cutcallback.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,8 @@ end

# CutCallbacks does not have child algorithms, therefore get_child_algorithms() is not defined

function get_storages_usage(algo::CutCallbacks, form::Formulation{MathProg.AbstractMasterDuty})
function get_storages_usage(algo::CutCallbacks, form::Formulation{Duty}
) where {Duty<:MathProg.AbstractFormDuty}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Nice! Sorry for this bug.

return [(form, MasterCutsStoragePair, READ_AND_WRITE)]
end

Expand Down
2 changes: 1 addition & 1 deletion src/Algorithm/branching/branchingalgo.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -179,7 +179,7 @@ function perform_strong_branching_with_phases!(
sort!(groups, rev = true, by = x -> (x.isconquered, x.score))

if groups[1].isconquered
nb_candidates_for_next_phase == 1
nb_candidates_for_next_phase = 1
end
Copy link
Collaborator

Choose a reason for hiding this comment

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

Artur, why did you remove this?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Hi Ruslan. I think it had no effect. Did you intend to use = instead of == ?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Anyway, we decided to let the nodes survive a little bit more before being deleted to let them appear in the branch tree file. We don't think there will be a significant impact in the performance.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Yes, it should be =. This is needed to avoid unnecessary evaluation of branching candidates, when one of candidates is already conquered.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I suggest marking it as conquered and testing it in the beginning of the evaluation to avoid evaluating it again but letting it persist until being written to the branch tree file (equivalent to BaPTree.dot). Otherwise some nodes will appear with only one child.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I did not completely get your point. Branching candidate (group) here is not a child node, but a complete set of child nodes (left branch and right branch). The group is already marked as conquered and will not be evaluated again. This is to avoid passing other candidates (or groups) to the next strong branching phase. Thus we avoid evaluating them. This evaluation is not needed as there exists a conquered branching candidate, i.e. all children nodes are conquered. We pass only this branching candidate, and all child nodes of this candidate will appear in the branch tree file.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Ah! ok. Got it. So, we should put the code back with = replacing ==.


# before deleting branching groups which are not kept for the next phase
Expand Down
7 changes: 7 additions & 0 deletions src/Algorithm/storage.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,13 @@ const StorageTypePair = Pair{DataType, DataType}
# be restored for writing (all other storages are restored anyway but just for reading)
const StoragesUsageDict = Dict{Tuple{AbstractModel, StorageTypePair}, StorageAccessMode}

function Base.show(io::IO, stoUsaDict::StoragesUsageDict)
Copy link
Contributor

Choose a reason for hiding this comment

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

Variable name must be lower case.

print(io, "storage usage dict [")
for usage in stoUsaDict
print(io, " (", typeof(usage[1][1]), ", ", usage[1][2], ") => ", usage[2])
end
print(io, " ]")
end

"""
function add_storage_pair_usage!(::StoragesUsageDict, ::AbstractModel, ::StorageTypePair, ::StorageAccessMode)
Expand Down
2 changes: 1 addition & 1 deletion src/Algorithm/treesearch.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -218,7 +218,7 @@ function print_node_in_branching_tree_file(
ncur = get_tree_order(node)
time = elapsed_optim_time(env)
if ip_gap_closed(getoptstate(node))
@printf file "\n\tn%i [label= \"N_%i (%.0f s) \\nPRUNED\"];" ncur ncur time
@printf file "\n\tn%i [label= \"N_%i (%.0f s) \\n[PRUNED , %.4f]\"];" ncur ncur time pb
else
@printf file "\n\tn%i [label= \"N_%i (%.0f s) \\n[%.4f , %.4f]\"];" ncur ncur time db pb
end
Expand Down
3 changes: 3 additions & 0 deletions src/Coluna.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,9 @@ const TOL = 1e-8 # if - ϵ_tol < val < ϵ_tol, we consider val = 0
const TOL_DIGITS = 8 # because round(val, digits = n) where n is from 1e-n
###

# submodules
export ColunaBase, MathProg, Algorithm

const _to = TO.TimerOutput()

export Algorithm, ColunaBase, MathProg, Env, DefaultOptimizer, Parameters,
Expand Down
2 changes: 1 addition & 1 deletion src/MOIcallbacks.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,7 @@ function MOI.submit(
end

constr = setconstr!(
form, "", MasterMixedConstr;
form, "", MasterUserCutConstr;
rhs = rhs,
kind = Essential,
sense = sense,
Expand Down
1 change: 1 addition & 0 deletions src/MathProg/duties.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,7 @@ end
MasterRepBendSpTechnologicalConstr <= AbstractMasterRepBendSpConstr
AbstractMasterCutConstr <= AbstractMasterConstr
MasterBendCutConstr <= AbstractMasterCutConstr
MasterUserCutConstr <= AbstractMasterCutConstr
AbstractMasterBranchingConstr <= AbstractMasterConstr
MasterBranchOnOrigVarConstr <= AbstractMasterBranchingConstr
AbstractDwSpConstr <= Duty{Constraint}
Expand Down