BuildingsPy icon indicating copy to clipboard operation
BuildingsPy copied to clipboard

Regression tests: Option to use reference results from custom path?

Open marcusfuchs opened this issue 4 years ago • 5 comments

From my understanding of the code in buildingspy/development/regressiontest.py, the reference results for the regression testing are always read from the following directory:

        # Check if the directory
        # "self._libHome\\Resources\\ReferenceResults\\Dymola" exists, if not
        # create it.
        refDir = os.path.join(self._libHome, 'Resources', 'ReferenceResults', 'Dymola')
        if not os.path.exists(refDir):
            os.makedirs(refDir)

We have a use case where we would like to use BuildingsPy to run tests on an auto-generated Modelica package. For this application, it would be useful if we could specify a base directory for the reference results which is outside of the generated library. This may e.g. be done by adding a new class attribute self.refDir and moving the code snippet mentioned above into the class initialization to execute it when the default refDir=None is passed to the ìnit` method.

As I understand that this proposed change may be a bit too special for all users I first wanted to ask whether there is an interest in me implementing this. As an alternative, we could also copy reference results into the library during the auto-generation process, so we could also work around this issue.

marcusfuchs avatar Nov 18 '20 08:11 marcusfuchs

@marcusfuchs : I think such an option would be useful. As requirements, we would like to be able to

  • specify a path for where the get data.
  • easily say to run the regression test with tool A but verify against time series from tool B. In this case, the size of the system of equations should not be compared (as they rely on tool-dependent heuristics).

mwetter avatar Nov 18 '20 17:11 mwetter

@mwetter Great, I can make a suggestion then.

I already saw that what I suggested above will not work correctly. It does not seem possible to set up the default reference directory path during initialization, because the path depends on the library root (self._libHome). This is not set at init, but in setLibraryRoot. So this is also where we have to set up the reference directory if no custom path is given. I'll open a PR as WIP for that.

Regarding your other point about comparing different tools, I am not yet sure how this would be best implemented. It seems to me like currently, the reference result directory is always refDir = os.path.join(self._libHome, 'Resources', 'ReferenceResults', 'Dymola') from which I assume that simulated results can currently only be compared against Dymola results? Unfortunately, it's been a while since I looked into other tools, so my understanding here may be too limited.

marcusfuchs avatar Nov 18 '20 19:11 marcusfuchs

@marcusfuchs : Thanks for the PR. Let me have a look at it and propose some edits for https://github.com/lbl-srg/BuildingsPy/issues/393#issuecomment-729815411

mwetter avatar Nov 20 '20 15:11 mwetter

@marcusfuchs : I made a working branch issue393_customrefs.

Can you please have a look at whether the new code meets what you need.

The following will still need to be done (most likely in December):

  • [ ] Write regression tests for the new functionality
    • compare with results outside of the library
    • compare results between Dymola and optimica for MyModelicaLibrary/Examples
    • write a tests that ensures failure if both new arguments base_reference_result_directory and base_reference_result_tool are specified.
    • write a test that ensures failure if base_reference_result_directory does not exist

mwetter avatar Nov 20 '20 16:11 mwetter

@mwetter Sorry that it took a little longer on my side, but now I had a look at your code. I think your additions still cover my use case nicely, so I think this is even better now.

Regarding the logic for the different options, I made some suggestions for a refactoring of your code. The functionality should not have been changed, but I find the conditional statements a little easier to understand in my version. In addition, I added 2 of the tests you suggested above, so what's left is the following

Write regression tests for the new functionality:

  • [ ] compare with results outside of the library
  • [ ] compare results between Dymola and optimica for MyModelicaLibrary/Examples
  • [x] write a tests that ensures failure if both new arguments base_reference_result_directory and base_reference_result_tool are specified.
  • [x] write a test that ensures failure if base_reference_result_directory does not exist

If you agree with my suggestions the current code would work for me. If you prefer your version, I could also revert my refactorings and just include the new tests.

My new code is already updated in https://github.com/lbl-srg/BuildingsPy/pull/394

marcusfuchs avatar Dec 01 '20 20:12 marcusfuchs