addVar is ambiguous in highspy with highs.py

[jajhall]

highspy contains addVar, but so does highs.py. It would appear that the method in highs.py is called, but this returns a variable, whereas addVar in highspy returns a HighsStatus (as in C++).

So, even if this precedence perpetuates, addVar in highspy cannot be called

Suggest changing all occurrences of "var" in highs.py to "variable" - and, since this is a full-length word, change all occurrences of "constr" to "constraint"

[mathgeekcoder]

Good call. There are a few options:

1. The user can explicitly call the base method via: super(highspy.Highs, h).addVar(...)
2. We can add a new method to highspy to call the base method, e.g., h._addVar(...)
3. We can rename highspy addVar to addVariable (or addContinuous since we also have addIntegral/addBinary)
4. We could change method signature so python can determine automatically, but I think this is confusing!

I don't mind any option. For backwards compatibility, I think (3) is best. However, if people are adding variables directly via the base class, it's not going to interact well if they're also trying to use the new highspy modelling features. In this case, options (1 or 2) is best so that the user explicitly knows what they're doing.

[mathgeekcoder]

Alternatively, we could rename the base class method addCol, to be similar to addRow, which implies low-level functionality.

[jajhall]

Thanks. I liked (2) for a moment, until I realised that - if I understand correctly - the call to Highs::addVar that is currently impossible, is achieved by calling h._addVar, breaking the naming convention in the wrapper.

Unfortunately, I can see people "maturing" from the simple modelling language by going to the base class calls to get additional functionality, so it's good if they can work side-by-side.

[jajhall]

Alternatively, we could rename the base class method addCol, to be similar to addRow, which implies low-level functionality.

... but this is tempting. There already is Highs::addCol, and Highs::addVar was added for people who didn't want to declare "just a variable x>=0" with

addCol(0, 0, inf, 0, nullptr, nullptr)

I've never been totally happy with this since, as you say, addCol and addRow imply low-level functionality.

[jajhall]

Removing Highs::addVar will break the API, and annoy people. However, since it can't be called from highspy, it might as well be removed from highs_bindings.cpp.

We just have to make sure that h.addVar behaves the same way as the method in highs_bindings.cpp.

[mathgeekcoder]

I totally agree that advanced users will likely want to access Highs directly, in which case they might prefer the low-level and bulk update calls anyhow.

Also, there is a bit more ambiguity here, since they can currently call addVars which directly calls Highs. We might want to wrap that too in highspy.