coin-or / MibS

A solver for mixed integer bilevel programs
Eclipse Public License 1.0
50 stars 20 forks source link

Add function to write aux file #106

Closed yuxies closed 1 year ago

yuxies commented 1 year ago

Added a function to write LL/SL/second stage problem into an auxiliary file; number style follows standard .mps format;

Will add/change

tkralphs commented 1 year ago

Thanks for this, but can you maybe remove the two commits that are not related? I guess you know how to do this, but if not, please feel free to ask.

The reading by name functionality is already there. See, e.g., https://github.com/coin-or/MibS/blob/10a55ae3804604ec73ab89022c9d8870986524b3/src/MibSModel.cpp#L410-L458

tkralphs commented 1 year ago

I guess maybe it wasn't clear that there is already a name-based format defined. I don't remember the details and perhaps it isn't well-documented. But it looks from the readInstance() that the idea is that a section starting with @VARSBEGIN indicates names rather than indices. I'm not sure if we might want to think about changing this.

tkralphs commented 1 year ago

See here for an example of an aux file with names.

tkralphs commented 1 year ago

Now looking at your code and reminding myself of how the file format was defined originally, I don't think what you're outputting is quite right. But the fix should be easy. Let me know if you don't know what I'm talking about.

yuxies commented 1 year ago

Oh I see. I was following this output format without section.

I will read the section file. But I am not sure how interdiction related coefficients (IB & IC) would be handled in this case. Maybe will find the answer somewhere.

tkralphs commented 1 year ago

Oh I see. I was following this output format without section.

Hmm, that is weird, I saw that file but read it as moore90WithoutName.txt. I don't see how that file can be read properly with the parser we have. I thought it needed the section names to know that it was a name-based file. But anyway, in the meantime, we had a discussion on Slack (I will get you added) about the specifications for this format, which we may as well further modify to be what we really want. So here are the results.

Does that makes sense? I guess it won't be too hard to implement this. I can help if needed.

yuxies commented 1 year ago

I change the writing format according to your last comment. But I am not sure the "end" line is necessary? Or was it used when N and M were not available?

Also, the output file name is now a MibS parameter. The parameter default is set to "", and for now the write function will only be activated when the input is nonempty.

I attached a sample output for you to look over. TestOutputFile.txt

tkralphs commented 1 year ago

We kind of decided to have the end marker, as a sanity check, even though it's not strictly necessary

tkralphs commented 1 year ago

For OBJSENSE, let's do MIN or MAX instead of "1" or "-1".

tkralphs commented 1 year ago

So how do I call MibS to read in an instance and then just write it back out in this format?

yuxies commented 1 year ago

So how do I call MibS to read in an instance and then just write it back out in this format?

The last commit is trying to read a parameter MibS_writeInstanceName here. The parameter default value is set to "", and if there is a nonempty input string, such as TestOutputFile.mps (assuming the file format is given as well, and the last 4 letters in the string will be removed later), then the program will write the MPS again (or make it optional?) and an aux file named TestOutputFile.aux.

Then the solver will run the regular solution process. I guess I can insert a termination statement into the write function if needed, so the parameter MibS_writeInstanceName will write the file and then exit immediately.

For OBJSENSE, let's do MIN or MAX instead of "1" or "-1".

I will update this.

tkralphs commented 1 year ago

OK, I got it working now. I somehow ended up running the stable version instead of this modified version, so it was not working. Yes, let's not write out the MPS file again and just use the same instance name by default, so that if we call

./mibs -Alps_instance ~/Projects/MibS/data/notInterdiction/Random/int0sum_i0_10.mps -MibS_writeInstanceName

we get int0sum_i0_10.aux in the working directory and the code just exits after that without solving.

tkralphs commented 1 year ago

I also noticed that the @MPS field has the full path to the original MPS file, but I guess it should just have the MPS file name (without the path) and it should be the name of the new file that was written out (if a name was specified), not the original MPS file name. In other words, I specified tmp.mps as the file name, but it will pointed to the original file that I read in instead.

yuxies commented 1 year ago

Yes, let's not write out the MPS file again and just use the same instance name by default

it should just have the MPS file name (without the path) and it should be the name of the new file that was written out (if a name was specified), not the original MPS file name

Okay, I will change the function to work like this:

Does it look right to you?

Also, should we keep the @NAME field in AUX file? For now it is always consistent with the name in @MPS.

tkralphs commented 1 year ago

Yes, what you outlined above sounds reasonable for now, but it is also possible for instances to specified by .lp files. Or at least I guess that it should be. I don't remember if we added this capability to MibS or not. I think we did, right? If not, we should, but if there is currently no support at all, then adding that could be a separate thing for another PR. If it's already possible to read .lp files in MibS, then let's support that here.

yuxies commented 1 year ago

it is also possible for instances to specified by .lp files

Is that the same as AMPL/GMPL support here?

yuxies commented 1 year ago

The last commit supports empty file name input by comparing the input string with the string PARAM_NOTSET.

To exit the program after files are written, I use the exception throw for a safe exit. I cannot think of a better way. Can you check to see if this makes sense?

Also, currently, @NAME is almost the same as @MPS. Can you remind me what we plan to do with @NAME?

tkralphs commented 1 year ago

it is also possible for instances to specified by .lp files

Is that the same as AMPL/GMPL support here?

No, it's not. Are you not familiar with LP format? https://www.ibm.com/docs/en/icos/12.8.0.0?topic=cplex-lp-file-format-algebraic-representation

There is a class in CoinUtils for reading files in LP format that has the same basic API as the class we're using here for reading MPS file. But I guess we are not actually using it in MibS. You can check SYMPHONY or Cbc to see how it's used (just grep for CoinLpIO). Let's separately implement that in another PR.

To exit the program after files are written, I use the exception throw for a safe exit. I cannot think of a better way. Can you check to see if this makes sense?

Why not just exit(0)?

Also, currently, @NAME is almost the same as @MPS. Can you remind me what we plan to do with @NAME?

That is because we're deriving the name from the file name of the MPS file automatically. But the intention is that this name would be a human-readable name with spaces, etc. We have the same option in MPS, actually, the instance can have a name string separate from the file name.

tkralphs commented 1 year ago

What I get now with

./mibs -Alps_instance ~/Projects/MibS/data/notInterdiction/Random/int0sum_i0_10.mps -MibS_writeInstanceName

is nothing at all (as before). If I do

./mibs -Alps_instance ~/Projects/MibS/data/notInterdiction/Random/int0sum_i0_10.mps -MibS_writeInstanceName abcdef

I get a file abc.aux and a file abcdef, with the latter being an MPS file, but the name not having the extension.

What I wanted was that if I give no file name, then I get int0sum_i0_10.aux and nothing else. If I give either xxx.mps or xxx.aux or xxx, then in all three cases, I should get two files: xxx.aux and xxx.mps.

yuxies commented 1 year ago

Why not just exit(0)?

I was not sure if exit(0) would clean up the stack and leave the program safely. But I can change that.

is nothing at all (as before).

Working on fixing this one (see comments on the code review above.)

I get a file abc.aux and a file abcdef, with the latter being an MPS file, but the name not having the extension

Was assuming the user would also provide the file extension with the name, as we may consider .lp files. I will change it to "when no extension is provided, assuming MPS file".

yuxies commented 1 year ago

The last two commits changed

  1. the naming of the output file when no extension in input value was detected;
  2. the trigger of the writeAuxFile function:
    • if the input value following the parameter is "0", then write the AUX file using the default name;
    • if the input value is anything else other than "0", then write both the MPS and AUX files using the input string;
    • if the input value is empty, the reading function will automatically drop the param, and nothing will happen,
    • and the default value "PARAM_NOTSET" is then kept to distinguish from "0" or other valid input strings.
tkralphs commented 1 year ago

I just went ahead and made the last remaining fixes for this. I change 0 to - as the indicator for default file name and also replaced the error with exit(0).