coin-or / python-mip

Python-MIP: collection of Python tools for the modeling and solution of Mixed-Integer Linear programs
Eclipse Public License 2.0
518 stars 92 forks source link

Unexpected file name on writing the solution #164

Open gt6989b opened 3 years ago

gt6989b commented 3 years ago

When calling model.write('myfile.mps'), the resulting output file myfile.mps.mps.gz is created.

  1. Please remove the invalid mps.mps suffix, the end result should be named myfile.mps.gz.
  2. It would likely be better if I were given an API option to choose if I want the result zipped or not. In my case, plaintext would be much better since I want to store this in a source control repository as a regression test, so now I have to add a step to unzip the file inside my process.

Thank you.


Running python 3.7 on mip-1.13.0 on both Windows and Linux.

ryanjoneil commented 2 years ago

Given this issue and #300, is it desirable to defer writing the model to Cbc? I think that, since the mip library knows the model definition, it could be responsible for writing (and potentially reading) well-structured .lp and .mps files.

Another issue with using Cbc for writing models to files is that its output is sometimes unreadable by other solvers. For example, if I try to write a .mps file with Cbc and read it with GLPK, I get this error:

% glpsol --mps coolmodel.mps 
GLPSOL--GLPK LP/MIP Solver 5.0
Parameter(s) specified in the command line:
 --mps coolmodel.mps
Reading problem data from 'coolmodel.mps'...
Problem: BLANK
coolmodel.mps:4: in fixed MPS format positions 13-14 must be blank
MPS file processing error

Writing .lp. and .mps files correctly should not be an enormous lift and would provide better interoperability.

ryanjoneil commented 2 years ago

In looking through SolverCbc.write, it should be easy to remove the repetitive .mps extension.

There are also a few bugs in the code that can be eliminated. For example, say we're a solving a model with the acronym "lumps". m.write("lu.mps.lp") will write a lu.mps.lp.mps.gz file instead of a .lp file.

I think the method should look like this. Happy to open a PR if you like.

    def write(self, file_path: str):
        fpstr = file_path.encode("utf-8")
        flower = file_path.lower()

        if flower.endswith(".mps"):
            # CBC adds a ".mps.gz" extension for us.
            cbclib.Cbc_writeMps(self._model, fpstr[:-4]) # remove .mps extension
        elif flower.endswith(".lp"):
            cbclib.Cbc_writeLp(self._model, fpstr)
        elif flower.endswith(".bas"):
            cbclib.Cbc_writeBasis(self._model, fpstr, CHAR_ONE, 2)
        else:
            raise ValueError(
                "Enter a valid extension (.lp, .mps or .bas) \
                to indicate the file format"
            )
sebheger commented 2 years ago

i would say this need to be fixed in the c-interface on cbc side. But there is nobody at the moment who will fix this at cbc side

tkralphs commented 2 years ago

As @sebheger said, it would be helpful if you would report these issues over at Cbc rather than here. I cannot guarantee anything, as the developer bandwidth is limited, but there is a good chance that low-hanging fruit like this would get addressed. A PR would also be more than welcome.

tusiqi1 commented 9 months ago

When will this bug be fixed?