Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix confusing error message in checkStage #581

Merged
merged 1 commit into from
Mar 26, 2022
Merged

Conversation

mattmilten
Copy link
Collaborator

fix #576

@mattmilten mattmilten merged commit 390cb35 into master Mar 26, 2022
@Joao-Dionisio
Copy link
Collaborator

Joao-Dionisio commented Jan 26, 2023

Hey @mattmilten , sorry for digging up an old merge. This is what master currently has:

if self.sol == NULL and not SCIPgetStage(self.scip) == SCIP_STAGE_SOLVING:
    raise Warning(f"{method} can only be called in stage SOLVING with a valid solution (current stage: {SCIPgetStage(self.scip)})")

But isn't this logically incorrect? The message is ambiguous, but I interpret it as saying that the method can only be called if we are in stage solving but with a valid solution (it can also be interpreted as "if the method is called in stage solving, it must have a valid solution", which I think is also wrong) . But the if condition allows for calling the method in the solving stage, even if we do not have a valid solution, right? Shouldn't the error message be:

if self.sol == NULL and not SCIPgetStage(self.scip) == SCIP_STAGE_SOLVING:
    raise Warning(f"{method} can only be called with a valid solution or in stage SOLVING (current stage: {SCIPgetStage(self.scip)})")

To put it another way, the current message is "stage solving implies valid solution", when it should be "null solution implies stage solving".

@mattmilten
Copy link
Collaborator Author

I suppose the if condition just needs to be changed to an OR instead of the AND. Then, the warning is raised whenever there is no solution available or we are not in the right stage.

@mattmilten
Copy link
Collaborator Author

Actually, this is correct because it should only guard against calling the two listed methods in that specific case.

@mattmilten
Copy link
Collaborator Author

Hmm, I am somewhat confused now. You should also check #493 for som added context. Maybe the check is not correct for all cases. I you think the error message needs a different wording, feel free to improve it.

@Joao-Dionisio
Copy link
Collaborator

Yeah, my issue was just with the error message, it wasn't the negation of the if condition when it must be. I think it should be

if self.sol == NULL and not SCIPgetStage(self.scip) == SCIP_STAGE_SOLVING:
    raise Warning(f"{method} can only be called with a valid solution or in stage SOLVING (current stage: {SCIPgetStage(self.scip)})")

instead. I'll get around to fixing it, eventually. I hadn't seen #493 , thanks! It seems to be related to #625 as well.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Cryptic error message
2 participants