Gurobi / gurobi-optimods

Data-driven APIs for common optimization tasks
https://gurobi-optimods.readthedocs.io
Apache License 2.0
153 stars 32 forks source link

Adapt OPF case parser for pglib files #133

Closed pobonomo closed 9 months ago

pobonomo commented 9 months ago

Those files have only the 10 first columns for the gen array. The others are actually not used in the formulations.

Description

This is to be able to read models from the power grid lib: https://github.com/power-grid-lib/pglib-opf

Checklist

This is not a new mod!

Have a nice day!

CLAassistant commented 9 months ago

CLA assistant check
All committers have signed the CLA.

JaromilNajman commented 9 months ago

@pobonomo Looks good.

simonbowly commented 9 months ago

Can you add a test case for this format, in the IO tests? https://github.com/Gurobi/gurobi-optimods/blob/3a28b2030584687abbca447990bd072388cd0582/tests/opf/test_io.py#L116-L148

pobonomo commented 9 months ago

Can you add a test case for this format, in the IO tests?

That's actually not something so simple.

The pglib is a benchmarking library for ACOPF. There is a report here:

https://arxiv.org/abs/1908.02788

they follow the same matpower format that is implemented in the optimod but they omit some columns that apparently are not useful for the formulation of ACOPF.

So to me it's not a new format. I don't think it's worth doing a new parser. What I did is just replacing the missing data with Nan.

Now using the current test is an issue because `float(nan) != float(nan)'. I could just replace the line that test that the data is different with a numpy assert that can handle nan I think:

@@ -135,7 +136,7 @@ class TestIO(unittest.TestCase):
                 # The first read and the round-trip should match exactly
                 self.assertEqual(set(reread.keys()), set(original.keys()))
                 for field, data in reread.items():
-                    self.assertEqual(data, original[field])
+                    np.testing.assert_equal(data, original[field])

                 # Check types for some special cases
                 for bus in reread["bus"]:

but to me it's making the test complicated and in the end less reliable.

On the other hand I think it is worth that the optimod code doesn't throw an error from a standard publicly available library.

simonbowly commented 9 months ago

On the other hand I think it is worth that the optimod code doesn't throw an error from a standard publicly available library.

This is really all I'm suggesting. If the optimod should handle these files, we should at least have a test that reads one, even if it doesn't assert anything about the result. That way, your new additions are actually covered by the tests and we're less likely to break this by some future change.

pobonomo commented 9 months ago

On the other hand I think it is worth that the optimod code doesn't throw an error from a standard publicly available library.

This is really all I'm suggesting. If the optimod should handle these files, we should at least have a test that reads one, even if it doesn't assert anything about the result. That way, your new additions are actually covered by the tests and we're less likely to break this by some future change.

You seem to have omitted the beginning of my message. The part of the file that we would use is already tested by the current test.

In any case I added a test specific for a file in the 'pglib' format.

simonbowly commented 9 months ago

Ok, sorry if I misunderstood something. Thanks for adding the test.