More LPI functions and optimality check

[lidingxu]

SCIP requires : Before calling SCIPlpiGetSol(), the caller must ensure that the LP has been solved to optimality, i.e., that SCIPlpiIsOptimal() returns true.

Optimality check is added for functions calling SCIPlpiGetSol(). For non-optimality call, None is returned. Memory issue in getRedCost() is fixed. More get functions are added.

[Joao-Dionisio]

I think it would be better to assert optimality rather than returning None. Otherwise, some minor grammar things and it looks good.

Liding, can you create a branch on the main repo instead of working on a fork? It makes things very slightly easier :)

[DominikKamp]

Could the optimality assertions be removed? The comment is just an unfortunate way to say that the user should make sure that there is a meaningful LP solution available. At least I am also interested in feasible primal and dual solutions.

[Joao-Dionisio]

@DominikKamp Oh... but wait, the comment doesn't seem unfortunate, it seems very explicit in its requirements.

Before calling this function, the caller must ensure that the LP has been solved to optimality, i.e., that SCIPlpiIsOptimal() returns true.

Do you know what made people write it like this? What would you change the comment to? Also, the C-side of the LPI needs to change, this cannot be just a comment in the documentation.

[DominikKamp]

The concern was

SCIPlpiGetSol() does not define what the output is if the primal or dual problem is unbounded or infeasible.

although primal and dual solutions have certain definitions. Then those comments were dumped (to scare users). If this would be a requirement, there were an assertion on the C-side, but this is not the case.

[Joao-Dionisio]

Fair enough, but I'd still be in favor of being more explicit in the C-documentation, and potentially adding some assertions. Not of optimality, but of the non-existence of the state that yields unexpected behavior. I'll make a PR removing the assertions now

[Joao-Dionisio]

@DominikKamp pushed directly to master 🤦

Does this comment work for you

Please note that infeasible or unbounded LPs might return unexpected results.

Or should I revert?

[lidingxu]

The problem is that LPI has a call to underlying LP solvers, something (like support of LPI param) is not defined there, thus leaving freedom to decide by C users, mostly developers. For pysciopt, I would see a better approach is not an assertion, we could change it to print a warning message "LP is not optimal", meaning that any reduced costs and duals are unreliable.

[Joao-Dionisio]

Oh, I like a warning much better. Can you prepare a PR @lidingxu?

[DominikKamp]

The comment is okay although I do not see why we should get unreliable results. The LP solver needs to prove its claims. For an unbounded LP I expect a feasible LP solution. For an infeasible LP I expect at least a farkas solution. If you then should encounter a wrong result, it needs to be documented and fixed.

[lidingxu]

For example, a primal simplex may not provide a feasible dual, if the problem is not solved optimal (if time limit is too small), the user needs a warning.

[DominikKamp]

If the user asks for an unavailable dual solution, an error makes sense. In this case there should still be a way to get an available primal solution. But should this check not happen in SCIP?

[lidingxu]

The functions of lpi is implemented separately in lpi_xxxsolver.cpp. For example, in SCIPlpiGetSol of lpi_clp.cpp, the reduced cost is obtained directly from the CLP library:

[SCIP_RETCODE](https://www.scipopt.org/doc-9.2.1/html/type__retcode_8h.php#ac847f5a370187651fbc1b82d0170b480) [SCIPlpiGetSol](https://www.scipopt.org/doc-9.2.1/html/group__LPIS.php#gac5a2315c14e99b919626de4170687227)(
   [SCIP_LPI](https://www.scipopt.org/doc-9.2.1/html/structSCIP__LPi.php)*             lpi,                /**< LP interface structure */
   [SCIP_Real](https://www.scipopt.org/doc-9.2.1/html/def_8h.php#a7f0ca7bf9e709814432691d8173a21fa)*            objval,             /**< stores the objective value, may be NULL if not needed */
   [SCIP_Real](https://www.scipopt.org/doc-9.2.1/html/def_8h.php#a7f0ca7bf9e709814432691d8173a21fa)*            primsol,            /**< primal solution vector, may be NULL if not needed */
   [SCIP_Real](https://www.scipopt.org/doc-9.2.1/html/def_8h.php#a7f0ca7bf9e709814432691d8173a21fa)*            dualsol,            /**< dual solution vector, may be NULL if not needed */
   [SCIP_Real](https://www.scipopt.org/doc-9.2.1/html/def_8h.php#a7f0ca7bf9e709814432691d8173a21fa)*            activity,           /**< row activity vector, may be NULL if not needed */
   [SCIP_Real](https://www.scipopt.org/doc-9.2.1/html/def_8h.php#a7f0ca7bf9e709814432691d8173a21fa)*            redcost             /**< reduced cost vector, may be NULL if not needed */
   )
{
   [SCIPdebugMessage](https://www.scipopt.org/doc-9.2.1/html/pub__message_8h.php#a827dc20f17fd394bf206451a2289292e)("calling SCIPlpiGetSol()\n");
 
   assert(lpi != NULL);
   assert(lpi->clp != [NULL](https://www.scipopt.org/doc-9.2.1/html/def_8h.php#a070d2ce7b6bb7e5c05602aa8c308d0c4));
 
   ClpSimplex* [clp](https://www.scipopt.org/doc-9.2.1/html/structSCIP__LPi.php#af3a357463d445234c9a4b4f9329a7e60) = lpi->clp;
   if( objval != [NULL](https://www.scipopt.org/doc-9.2.1/html/def_8h.php#a070d2ce7b6bb7e5c05602aa8c308d0c4) )
      *objval = clp->objectiveValue();
 
   if( primsol != NULL )
   {
      const double* sol = [clp](https://www.scipopt.org/doc-9.2.1/html/structSCIP__LPi.php#af3a357463d445234c9a4b4f9329a7e60)->getColSolution();
      BMScopyMemoryArray( primsol, sol, clp->numberColumns() );
   }
   if( dualsol != [NULL](https://www.scipopt.org/doc-9.2.1/html/def_8h.php#a070d2ce7b6bb7e5c05602aa8c308d0c4) )
   {
      const double* dsol = clp->getRowPrice();
      BMScopyMemoryArray( dualsol, dsol, clp->numberRows() );
   }
   if( activity != [NULL](https://www.scipopt.org/doc-9.2.1/html/def_8h.php#a070d2ce7b6bb7e5c05602aa8c308d0c4) )
   {
      const double* act = clp->getRowActivity();
      [BMScopyMemoryArray](https://www.scipopt.org/doc-9.2.1/html/memory_8h.php#a7803953aee6994e3f513d4dcf9a2bf3a)( activity, act, clp->numberRows() );
   }
   if( redcost != NULL )
   {
      const double* red = clp->getReducedCost();
      [BMScopyMemoryArray](https://www.scipopt.org/doc-9.2.1/html/memory_8h.php#a7803953aee6994e3f513d4dcf9a2bf3a)( redcost, red, clp->numberColumns() );
   }
 
   return SCIP_OKAY;
}

SCIP seems not check the availability at all, I do not know whether the clp will check.

[DominikKamp]

Every LP solver should indicate whether the respective solution is available and based on this the interface can return an error if an unavailable solution is accessed. So a change in SCIP is definitely required as shown by this example. But this is individual to each LP solver, where it would make sense to start with SoPlex since we can maximize utility there.