Closed spoorendonk closed 5 months ago
Looks like this PR is not building. I've never actually used std::function
and I'm not sure exactly what's wrong. To me, this doesn't look a whole lot easier than creating the derived class, but maybe it will be for some. I wonder whether we could/should have an even simpler solution that doesn't even require use of the DecompApp
class. If your goal is to create a C interface, couldn't the methods for setting callbacks be pure C (global functions, not in a class)? Or would we have such functions as part of the C interface, but have the functions with the DecompApp
class for C++?
One callback to do everything is appealing and shouldn't be too difficult for users to figure out if one provides a template. In SYMPHONY, the callbacks are done using template functions that are always called, but can be empty. The user just fills in the existing functions if desired. That is another possible approach.
The callback class is essentially what we already have so I don't see the advantage of that.
PR is not building because I broke the code by removing virtual
from the DecompApp
methods. Just to illustrate what the approach would look like.
For the C interface I would wrap C user functions inside C++ callback objects to allow multiple DecompApp
with different callback functions. The user would just do
// set
Dip_DecompApp_setCallback(app, callback);
// unset
Dip_DecompApp_setCallback(app, NULL);
for an app and callback. There would be a global registry of app-callback relations for bookkeeping.
I am not sure how to avoid DecompApp
altogether for callbacks without some global variables which would make it hard to have multiple instances running concurrently?
From a user perspective I think one callback to do all is easier - Gurobi uses a where
and ẁhat
argument to guide control flow. However, then all return values from current functions needs functionality to be set from the callback, like add vars, cuts, and solutions. Maybe not too bad.
Suggestion:
void callback(DecompApp* app, DecompWhere where)
{
if(where == DecompGenerateCuts){
double *x = app->getAlgo()->getXhat();
DecompCutList newCuts = generateUserCuts(x);
app->getAlgo()->addCutsToPool(x, newCuts, newCuts.size());
}
if(where == DecompGenerateVars){
// add vars
}
}
The callback can be called directly from DecompAlgo
as a supplement to the member functions of DecompApp
or we could remove member functions and have only a callback function pointer that can be set by user? I think this is less code and perhaps easier to work with as a user?
OK, I see the issue with global callbacks. Actually, we have run into a related difficulty with the SYMPHONY design, now that you mention it. One callback that does everything works and makes it easy to add callbacks later without changing the API.
If we just have one callback function, I'm not sure there's too much difference between it being a member function in DecompApp
and just having a pointer to it in DecompApp. I guess the difference from a technical standpoint would just be whether the function has access to private data members. But there really aren't any. Actually, looking at it now, I'm not sure why most of the data members are public. Maybe just for convenience. Could think about whether to change that. For now, I guess leaving it as a member function makes sense. We should look at the rest of the member functions in DecompApp to see if it really makes sense for them to be there.
By the way, there are some TODOs sprinkled around that it might make sense to look at while we're doing some re-thinking. Also some open issues.
I am working on a solution with one callback function. For now a virtual function in DecompApp that calls a functions pointer if initialized. This way you can do a callback function in a derived DecompApp or set it from the outside using just the DecompApp. The latter makes it easier to do port it to C.
The design is very close to how Gurobi does it using where
and what
arguments. I think it is fairly easy to understand and it easy to extend with new callback functionality.
An example callback function would be:
void myCallback(DecompApp* app, DecompCallbackWhere where, DecompCallbackData& data){
std::cout << "myCallback " << where << std::endl;
if(where == DecompCallbackWhere::MIPSOL_FEAS){
const int numCols = data.getInt(where, DecompCallbackWhat::MIPSOL_NUMCOLS);
const double* x = data.getDoubles(where, DecompCallbackWhat::MIPSOL_X);
const double tolZero = data.getDouble(where, DecompCallbackWhat::MIPSOL_TOLZERO);
// dostuff
bool isFeasible = true;
data.setSolutionStatus(where, DecompCallbackWhat::MIPSOL_STATUS, isFeasible);
}
if(where == DecompCallbackWhere::MIPSOL_HEUR){
const double* xhat = data.getDoubles(where, DecompCallbackWhat::MIPSOL_X);
const double* origCost = data.getDoubles(where, DecompCallbackWhat::MIPSOL_XOBJ);
// dostuff
DecompSolution *sol;
data.setSolution(where, DecompCallbackWhat::MIPSOL_ADD, sol);
}
if(where == DecompCallbackWhere::INITVARS){
// dostuff
DecompVar *var;
data.addVar(where, DecompCallbackWhat::MIPVARS_ADD, var);
}
if(where == DecompCallbackWhere::MIPVARS){
const int whichBlock = data.getInt(where, DecompCallbackWhat::MIPVARS_BLOCK);
const double* redCostX = data.getDoubles(where, DecompCallbackWhat::MIPVARS_REDCOSTX);
const double target = data.getDouble(where, DecompCallbackWhat::MIPVARS_TARGET);
// dostuff
DecompVar *var;
DecompSolverStatus status = DecompSolStatNoSolution;
data.addVar(where, DecompCallbackWhat::MIPVARS_ADD, var);
data.setSolverStatus(where, DecompCallbackWhat::MIPVARS_ADD, status);
}
if(where == DecompCallbackWhere::MIPCUTS){
const double* x = data.getDoubles(where, DecompCallbackWhat::MIPCUTS_X);
// dostuff
DecompCut *cut;
data.addCut(where, DecompCallbackWhat::MIPCUTS_ADD, cut);
}
}
Decomp app;
app.setCallback(myCallback);
I suggest to have a derivation of DecompCallbackData
for each case, e.g., DecompCallbackDataVars
to be have smaller objects. Maybe it is overkill ...
It will be behind the scenes so no user confusion.
I suggest to skip functions initDualVector
, getDualForGenerateVars
, solveRelaxedWhich
and solveRelaxedNest
. Non of the examples uses these, and we can always put them back in.
To port existing code, we can simply call the current derived functions from a new callback function, it should be ok.
Sounds good!
I will move forward with this approach
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.
A simple solution to how callback functions can be used in
DecompApp
.This approach uses
std::function
and sets them directly per method that can do callback. The example is for cuts and solving the relaxations. There can only be one method assigned per callback so multiple cut generators or pricing algorithms must be handled by the one callback being set.I am not entirely happy with the approach since you have to make a new setter for each type of callback.
Also, a thing with
std::function
and setting member functions has by usingstd::bind
:not the cleanest approach, but doable.
Alternative solutions are
where
argument.DecompApp
. Less flexible for prototyping where on wants to add global functions fast.@tkralphs any thoughts one how to approach this?