Cbc command line ignores "seconds"

[jhmgoossens]

The latest build of Cbc (command line tool) and CbcMain ignore the "seconds" time limit argument.
If this is a known remaining TODO on the list for the Cbc refactoring #374, then please close & ignore this issue.

The "seconds" option seems to be recognized:
seconds was changed from 1e+08 to 5
but is not actually used during the run--the solve just keeps going--because the dblParam CbcMaximumSeconds is not set to the requested value.
Example: cbc -import mas74.mps -seconds 5 -solve -quit

What does work:

• Directly setting cbcModel's setMaximumSeconds(5.0) works fine though.
• Similarly, cbcParameters[CbcParam::TIMELIMIT]->setDblVal(5.0) works well.

I tried cbc -import mas74.mps -timelimit 5 -solve -quit
but that fails with Unrecognized parameter.

[jhmgoossens]

Actually, using CbcModel's setMaximumSeconds( ) doesn't work because CbcParam::TIMELIMIT value (and similar) overrides CbcModel settings when CbcMain1 calls parameters.synchronizeModel() which copies values from CbcParams to CbcModel. The only way to go seems to be to use CbcParam::TIMELIMIT . Or am I getting this wrong?

[tkralphs]

Sorry @jhmgoossens I somehow missed your original report when it came in. Yes, this is a sticky point that needs to be addressed somehow. Ideally, we would unify the parameter mechanisms in CbcSolver and CbcModel, but as a quick fix, the synchronization should take place when the parameter is actually set in CbcSolver, not later on. What is happening (I guess) is that the parameters are set in CbcSolver, then you are changing a parameters in CbcMode, and only afterwards is the synchronization occurring (right prior to the solve call).

For a quick fix, we can incorporate the sychronization into the parameter setting itself, which was the original intent. In fact, there is a push function associated with parameter that gets called when the parameter is set in order to do whatever lese is necessary besides just recording the value. So far, those functions are not used, but could be used for this purpose. Maybe we can work together on a quick fix that works for now and then do something better in the long-run?

[jhmgoossens]

Ok, two-way pushing of changes would be a way to resolve this until a more rigorous refactoring. I'll try to understand what's going on.
Putting such parameters into the method call to solve (or CbcMain) looks like good design (similar to Cplex solve with parameterset array), since it shows to the user that these are given for the rest of the solve process--not properties of an object that might be changed by the user at any time on another thread, with mixed results.
Are there parameters that have a good use case for values to change mid-solve, like in a callback?

[tkralphs]

I don't think two-way pushing is necessary, since it wasn't that way before. We just need to ensure that the pushing into CbcModel occurs right away and not after one of the parameters was subsequently set to a different value that was really meant to over-ride the first one. One of the things I had in mind with the new parameters class was CbcModel and CbcSolver could possibly just share the same parameter object. That just eliminates the issue altogether. I think it would not be too difficult to achieve this, but it would probably involve a lot of rote substitution and could end up breaking a lot of old code unless we leave the parameter setting functions of CbcModel in place and just change their implementation.

There are certainly parameters I could imagine a user wanting to change in a callback (changing frequency of cut generation or primal heuristics, for example). But it would also seem to be easy to allow this with the parameter object.

[tkralphs]

Actually, it looks like most of the mechanics are already there, just completely untested. The parameters already have push functions defined and the model parameters all share a common push function. This is set here.

Cbc/src/CbcParameters.cpp

Lines 2354 to 2362 in 41709ce

	for (int code = CbcParam::FIRSTMODELPARAM + 1;
	code < CbcParam::LASTMODELPARAM; code++) {
	if (getParam(code)->type() == CoinParam::paramInt){
	getParam(code)->setPushFunc(CbcParamUtils::pushCbcModelIntParam);
	}else{
	getParam(code)->setPushFunc(CbcParamUtils::pushCbcModelDblParam);
	}
	}
	}

The implementation of pushCbcModelIntParam is here.

Cbc/src/CbcParamUtils.cpp

Lines 1219 to 1274 in 41709ce

	int pushCbcModelIntParam(CoinParam &param)
	{
	CbcParam &cbcParam = dynamic_cast<CbcParam &>(param);
	CbcModel *model = cbcParam.model();
	int val = cbcParam.intVal();
	int cbcParamCode = cbcParam.paramCode();
	assert(model != 0);
	int retval = 0;
	/*
	Translate the parameter code from CbcParamCode into the correct key
	for CbcIntParam, or call the appropriate method directly.
	*/
	CbcModel::CbcIntParam key = CbcModel::CbcLastIntParam;
	switch (cbcParamCode) {
	case CbcParam::CUTPASS: {
	model->setMaximumCutPassesAtRoot(val);
	break;
	}
	case CbcParam::LOGLEVEL: {
	CoinMessageHandler *hndl = model->messageHandler();
	assert(hndl != 0);
	hndl->setLogLevel(val);
	break;
	}
	case CbcParam::MAXNODESNOTIMPROVING: {
	model->setMaxNodesNotImproving(val);
	break;
	}
	case CbcParam::MAXSOLS: {
	model->setMaxSolutions(val);
	break;
	}
	case CbcParam::NUMBERBEFORE: {
	model->setNumberBeforeTrust(val);
	break;
	}
	default: {
	std::cerr << "pushCbcModelIntParam: no equivalent CbcIntParam for "
	<< "parameter code `" << cbcParamCode << "'." << std::endl;
	retval = -1;
	break;
	}
	}
	if (key != CbcModel::CbcLastIntParam) {
	bool result = model->setIntParam(key, val);
	if (result == false) {
	retval = -1;
	}
	}
	return (retval);
	}

So the pushing is already set up for some of the parameters. One would just need to check that this mapping is correct. Currently, the push functions are off by default. To do the actual pushing, there is an optional parameter to the set methods of CoinParam, like here.
https://github.com/coin-or/CoinUtils/blob/79344533de4c7d57315ad75b08cc834614fdd8dd/src/CoinParam.hpp#L341-L343
One potential gotcha is that it is necessary for a pointer to the model to set prior to calling the push function. This is done with

Cbc/src/CbcParameters.hpp

Line 1849 in 41709ce

inline void setModel(CbcModel *model) { model_ = model; }

[jhmgoossens]

Finally made time to investigate this. But now I realize that f462681 resolved this issue "for now"! Thanks @tkralphs.

[tkralphs]

Yes, but that is a workaround that I would like to get rid of, so it would still be great if you could help investigate and do some testing!