Parsing objective sense in MPS file#656
Parsing objective sense in MPS file#656stephenjmaher wants to merge 3 commits intocoin-or:masterfrom
Conversation
|
Stephen J. Maher seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account. You have signed the CLA already but the status is still pending? Let us recheck it. |
1 similar comment
|
Stephen J. Maher seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account. You have signed the CLA already but the status is still pending? Let us recheck it. |
pchtsp
left a comment
There was a problem hiding this comment.
Thanks for the update. I'm still unsure if we should where we should check for None and where to assign the mode of the line. I've left some comments.
| parameters['sense'] = const.LpMaximize if line[1] == 'MAX'\ | ||
| else const.LpMinimize |
There was a problem hiding this comment.
Aren't you missing a mode=CORE_FILE_OBJSENSE_MODE here too?
There was a problem hiding this comment.
Not in this case. This line is catching the case where you have the objective sense provided in the same line as the keyword OBJSENSE, such as
OBJSENSE MAX
In this case we don't need to enter the OBJSENSE mode.
| else: | ||
| mode = CORE_FILE_OBJSENSE_MODE |
There was a problem hiding this comment.
I'm not sure why would we have the same mode regardless of the if clause. Maybe we should put the assignment before the len(line)>1 and take out the else clause? Or maybe I don't understand the idea.
There was a problem hiding this comment.
Following on from the previous comment, we have to enter the CORE_FILE_OBJSENSE_MODE is the objective sense is not given in the same line as the OBJSENSE keyword. This is catching the case
OBJSENSE
MAX
The check of len(line) > 1 differentiates between these two cases.
| elif mode == CORE_FILE_OBJSENSE_MODE: | ||
| parameters['sense'] = const.LpMaximize if line[0] == 'MAX'\ | ||
| else const.LpMinimize |
There was a problem hiding this comment.
shouldn't we test if parameters['sense'] is None? or if we provided an argument to the function (i.e., via the sense argument?
There was a problem hiding this comment.
We could put in a check here for whether parameters['sense'] is None. However, in the current code the check is not necessary since it is only possible to enter the CORE_FILE_OBJSENSE_MODE mode if parameters['sense'] is None.
We could put in an assert or throw an exception to protect against future code changes? Something like (after line 154)
if parameters['sense'] is not None
raise Exception("The supplied objective sense will be overwritten by the MPS file objective sense")
Fixes #655
This PR makes it possible to set the objective sense in an MPS file. Currently, the objective sense in the MPS file is ignored. The sense needs to be supplied to
fromMPSas a parameter.This change does the following:
senseparameter inLpProblem.fromMPSnow defaults toNone.readMPSmethods checks for the headerOBJSENSEin the MPS file. If the header is encounter there are two options:or
both options are handled in the
readMPSmethod.LpMinimize.LpProblem.fromMPS, then this overrides the sense that is specified in the MPS file.