Should we rather error out if DB is not the latest SpineOpt version?

[manuelma]

At the moment we just issue a warning saying that SpineOpt might still be able to run but results are not guaranteed. Note that this is not the same warning as the "missing elements" warning that we issue whenever the DB is missing some elements from the template - this latter case is just fine I believe.

But if the DB version is wrong it means the data probably requires some transformation (which is what the migration functions do) for SpineOpt to work right, so we should error out and tell the user to run with upgrade=true to run the migration - while clearly stating that migration is irreversible so they should backup their data.

An alternative would be to make upgrade=true the default - but that means we'd be irreversibly modifying user data without their consent, and then if the migration function happens to have an error we'd be in big trouble.

[tarskul]

A softer option is to adjust the current warning and state not only that the results are not guaranteed but also that we really recommend to run with upgrade=true (but maybe that is already the case). Because maybe they are collaborating and have slightly different versions that still produce the expected results?

But I'm also not against the idea of throwing an error.

[manuelma]

The problem - I think - is the warning gets swarmed by the model building output ("creating variable so and so", etc) so the users tend to miss it. It needs to be something more disruptive.

[tarskul]

We could also make the output more clear. My suggestion would be to clear the console output at the end (only at the end such that the user can still see the warnings while SpineOpt is running) and then print a formatted summary. For example as below. The status is at the very end such that it is the first thing they see.

########
VARIABLES
########
blablabla

########
CONSTRAINTS
########
more blablabla

#######
RESULTS
#######
objective value = ..

########
STATUS
########
spine db version: OK
solver status: Optimal

OVERALL STATUS: OK

But again, this is only an idea, I'm not opposed to throwing an error.

[jkiviluo]

There could also be two categories of warnings, critical and normal. Tars' summary could then distinguish how many warnings of each type there were and highlight if there were critical warnings. (with the warnings themselves listed earlier)

But yes, might be best to thrown an error in this case.

[nhniina]

Related to this, we have previously discussed that indeed, the upgrade could be changed by default to true but a backup copy of the old DB should be created. We should also be careful with online DBs: if the DB is online, the default upgrade=true could damage the DB for everybody else. We could also add a prompt if not in the latest version, with yes/no options to the upgrade. In addition, we could add a prompt to make a backup (yes default, but for large databases you may not want that). It should also be possible to run SpineOpt also without all the prompts.

[DillonJ]

Yeah, I think user prompt with backup might be the best way. If the argument to upgrade is explicitly provided, then could be a warning but no prompt to allow for commandline execution where responding to prompts would be problematic.

Regarding the output, what I have thought would be really helpful is static non-scrolling output that gives you information about the current status, rather than having to scroll through the console which is difficult when it's updating. The idea would be to have one or two lines of static output giving the current benders iteration/window/elapsed time etc. and then the normal scrolling output below that. Not sure this is even possible, but I'm sure I've experienced this behaviour before

Here's a reddit thread that discusses the static + scrolling output: https://www.reddit.com/r/csharp/comments/2ec8wz/console_application_staticnot_scrollable_lines/

[manuelma]

The prompt idea is interesting but not straightforward to implement. We would need a mechanism for a tool like SpineOpt to tell toolbox to issue a prompt and then communicate back the result - not easy at all.

[manuelma]

The non-scrollable output idea is also good but also difficult to implement at toolbox level because we'd need to handle some new ansii escape characters in our console widget - again, not too easy.

[manuelma]

The best compromise in my opinion is to keep upgrade=false by default but then error out if the DB is not the latest version while telling the user to rerun with upgrade=true (making sure they understand the risks including shared DBs etc.)