oemof-solph icon indicating copy to clipboard operation
oemof-solph copied to clipboard

Replace constraint tests

Open p-snft opened this issue 8 months ago • 5 comments

Currently, we use constraint tests to check if a component/ constraint produces the correct lp file. In fact, they are not unit tests but integration tests, including problems associated with that.

  1. The LP files depend on Pyomo internals, thus they change when Pyomo changes things. Also, they are not valid for (hypothetical) other back ends. In the past, we had to change, e.g. because Pyomo changed rounding between binary and decimal representations. Because of race conditions in Pyomo, at the moment, we just test if old and new lp file contain the same lines, not their order. (See https://github.com/oemof/oemof-solph/commit/faf3c8658fd8a004db1edb59c1d51db374fc3893 to understand that the tests require meaningless changes from time to time.)
  2. They include tests for stuff from multiple parts of the code. For example, if I change the constraint formulation for the Bus, every constraint test needs to be rewritten, not only the one for the Bus.
  3. They are hard to understand (for many) and failed tests are hard to fix. The latter is particularly because of the undefined order of lines, so that just looking at a diff will not tell the difference: Constraints might be in a different order but still the same.

I would suggest to only have unit tests, in the meaning that when you change something in the code, only one corresponding test (or a handful of tests) needs to be rewritten. For example, the tests for components might test their characteristic curves. It will be driven by one of the flows being fixed at a time and check if the other flows show the expected value. This kind of test

  1. will be using Pyomo but the results will be independent from the backend.
  2. is easy to understand and
  3. even internal constraint formulations can be changed without breaking the test, so it really tests for the specified functionally not the implementation.

p-snft avatar Jun 11 '24 18:06 p-snft