Add possibility for nonlinear objective function

[Joao-Dionisio]

It's passing all the tests, currently the only issue is that calling getObjective isn't as nice as with a linear objective.

I deleted an assertion that I'm not sure what its purpose was, hopefully it was only to prevent nonlinear objectives, which should be alright now.

EDIT: just realized that this was on the wrong branch...

[codecov]

Codecov Report

Attention: Patch coverage is 23.80952% with 16 lines in your changes are missing coverage. Please review.

Project coverage is 52.30%. Comparing base (30148ab) to head (ed3de60).

Files	Patch %	Lines
src/pyscipopt/recipes/nonlinear.py	0.00%	11 Missing ⚠️
src/pyscipopt/scip.pxi	50.00%	5 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master     #781      +/-   ##
==========================================
+ Coverage   51.92%   52.30%   +0.37%     
==========================================
  Files          18       19       +1     
  Lines        3892     3904      +12     
==========================================
+ Hits         2021     2042      +21     
+ Misses       1871     1862       -9     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[Opt-Mucca]

This is on my to-do list. I remember there was an issue with adding such a function in previous versions, but can't remember what that issue was.
I wouldn't be comfortable merging this until @fserra or @mattmilten explain why non-linear objectives weren't supported previously.

[mattmilten]

I have no clue, really. Maybe we were just too scared back then ;-)

[fserra]

I think we might have thought of 2 things:

1. We were always afraid of making pyscipopt deviate too much from SCIP. I guess this qualified as deviating too much
2. I haven't looked at the MR but one would need to add an extra variable. I guess we were afraid about the fact that now we have to deal with this variable somehow... maybe the user doesn't understand why there is an extra variable in the problem. I feel/felt that forcing the user to add the variable themselves would give the user more control.

I think that was it. Anyway, that was back then. I let you guys decide if any of that makes sense.

[Opt-Mucca]

@Joao-Dionisio I added some changes to the branch.
Summarised version with some tips for the test:

• Avoid using randomness in tests unless a random seed is used
• I think the test was very incorrect regardless of the randomness
• I added checks for GenExpr and the corresponding other NL structures
  I'm still not sure if I actually want to merge this. I'm on the fence, and agree with Felipe that this diverts from simply being a modelling framework for SCIP. I foresee many headaches with supporting this feature and the intermediate variables + constraints that it creates.

[Opt-Mucca]

@Joao-Dionisio @mmghannam I slept on this, and am now firmly against adding such functionality. It requires keeping track of some hidden overhead, and I think will confuse more users than help. I think we should just add an appropriate error warning when a user tries to add a non-linear objective.
This then isn't too confusing for new users (if the error warning includes a modelling tip)

[Joao-Dionisio]

Hey @Opt-Mucca, fair enough. I think the main point is that the required reformulation isn't obvious for new users, so the modeling tip would be a must, in my opinion, maybe even explicitly saying which constraint to add. I've come across a fair amount of people asking how to have a nonlinear objective: #663, #387, #212, #168, #167, #41, SO1, SO2, ORSE1.

For posteriority's sake, I'll leave my counterarguments, but if we elaborate on the warning as you said, then I'm fine with closing.

• Having the option to add a nonlinear objective directly doesn't exclude the ability to not do it. So if the user wants more control, he can still model things as he pleases.
• I think it's reasonable to assume that someone who is bothered by this change is more likely to know about the reformulation and find ways to do what he wants (which this PR wouldn't impact in any way), than a less experienced user to find out about the reformulation by himself.
• I don't know what restrictions we have in place for naming variables and constraints, but we could name the new variable as something like "Variable modeling the nonlinear objective - see [some sort of reference]". Also, we are only adding one variable and one constraint. We could also add a message saying that we did this so that users aren't unaware.
• PySCIPOpt is used in a couple of classes, I think we can say that inexperienced users have a higher weight here than people who work with SCIP proper (excluding those using it as a black box), so having a slight difference between PySCIPOpt and SCIP doesn't bother me immensely (but I admit it might be my own lack of experience showing).

As for the randomness in the tests, I figured that a less deterministic test would be better, as we would be testing a wider array of problems, and possibly catching an error that we wouldn't otherwise (again, possible lack of experience showing).

An additional question. SCIP already handles nonlinear objectives, right? Can we not just pass it over somehow?

(I know I need to work on my conciseness, Mark, sorry :( )

[mmghannam]

I think both your arguments/concerns @Joao-Dionisio, @Opt-Mucca & @fserra make sense. This issue is similar to what's happening in #776 too. On the one hand the feature would help new users get things working quickly, but they are also not mapping to a feature in SCIP and could include judgement calls on how they are implemented. Also, including them as an example, or a warning message would be less convenient for users. So I suggest a somewhat middle ground solution, we could add a sub-package called maybe recipes, and it would include these type of functions. If it's implemented as external functions (that are not methods on the model). It will make it clearly separate from SCIP's API while still allowing a place for contributions for convenience methods/formulations.