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

Read in zero values from cplex #215

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Read in zero values from cplex #215

wants to merge 1 commit into from

Conversation

willu47
Copy link
Member

@willu47 willu47 commented Feb 1, 2024

Reads in zero values from the CPLEX solution file

Description

Removes the piece of code which excluded zero values from the CPLEX solution.

Issue Ticket Number

#214

Documentation

This has not been documented.

@willu47
Copy link
Member Author

willu47 commented Feb 1, 2024

A few things to consider before merging:

  • performance -> how big are the CSV files going to be if they contain zero values?
  • adding a command line flag to make this a user-defined option

@willu47 willu47 requested a review from trevorb1 February 1, 2024 13:00
@trevorb1
Copy link
Member

trevorb1 commented Feb 1, 2024

Hi @willu47! I believe your second bullet point of

adding a command line flag to make this a user-defined option

has been implemented in PR #126! Specifically, the --write_defaults flag. However, I just tried to do a test with it on Simplicity, and the default values were not written out :(

When I run this command

otoole results cplex csv model.sol results-with-zero datafile data.txt config.yaml --write_defaults

The results write out with zeros removed. No helpful debugging logs got printed out either. I tried with cbc as well, and the same thing happened; results write out with the zeros removed.

@trevorb1
Copy link
Member

trevorb1 commented Feb 1, 2024

Ahh.. we are hitting the KeyError exception here. The dubugging statement is just lousy cause it printed out the key REGION rather then something more descriptive. My fault there! haha

otoole/src/otoole/input.py

Lines 260 to 264 in 15bdc0b

if self.write_defaults:
try:
self.input_data = self._expand_defaults(inputs, default_values)
except KeyError as ex:
logger.debug(ex)

Copy link
Member

@trevorb1 trevorb1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please see PR #216 for my suggestion on how to address reading in zeros from CPLEX. I would vote to leave the removal of zeros in the ReadCplex class because:

  • Be consistent with how other solvers present results
  • This is probably the common desired functionality, so to reduce size of dfs (like you mentioned) when handling data.
  • Use the existing WriteStrategy._expand_defaults() class to achieve the same result

But! I am not opposed to this solution if you (or others) feel we should be presenting unmodified cplex results to the user! :)

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.

2 participants