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

Adapt OPF case parser for pglib files #133

Merged
merged 2 commits into from
Mar 4, 2024
Merged

Conversation

pobonomo
Copy link
Member

@pobonomo pobonomo commented Feb 21, 2024

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!

  • Implementation:
    • Implementation of the Mod in the gurobi_optimods installable package
    • Tests for the Mod implementation in tests/
    • Docstrings for public API, correctly linked using sphinx-autodoc
  • Documentation page:
    • Background and problem specification
    • Example of the input data format (use gurobi_optimods.datasets for loading data)
    • Runnable code example
    • Presentation of solutions
    • Included in the mod gallery and toctree

Have a nice day!

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

CLAassistant commented Feb 21, 2024

CLA assistant check
All committers have signed the CLA.

@pobonomo pobonomo self-assigned this Feb 21, 2024
@JaromilNajman
Copy link
Contributor

@pobonomo Looks good.

@simonbowly
Copy link
Member

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

def test_read_case_matpower(self):
# Check that all example cases are read without errors
case_mat_files = [
self.dataset_dir.joinpath(f"case{case}.mat")
for case in ["9", "14", "57", "118", "300", "NY"]
]
for file_path in case_mat_files:
with self.subTest(file_path=file_path):
# Should read without errors
original = read_case_matpower(file_path)
# Test write and read back
with tempfile.TemporaryDirectory() as tmpdir:
tmpfile = pathlib.Path(tmpdir) / "testcase.mat"
write_case_matpower(original, tmpfile)
reread = read_case_matpower(tmpfile)
# 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])
# Check types for some special cases
for bus in reread["bus"]:
self.assertIsInstance(bus["bus_i"], int)
self.assertIsInstance(bus["type"], int)
for gen in reread["gen"]:
self.assertIsInstance(gen["bus"], int)
for branch in reread["branch"]:
self.assertIsInstance(branch["fbus"], int)
self.assertIsInstance(branch["tbus"], int)

@pobonomo
Copy link
Member Author

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
Copy link
Member

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
Copy link
Member Author

pobonomo commented Mar 1, 2024

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
Copy link
Member

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

@simonbowly simonbowly merged commit 2cc5fe0 into Gurobi:main Mar 4, 2024
10 checks passed
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.

4 participants