lbl-srg / BuildingsPy

Python modules for automating Modelica simulations and for running unit test for the Buildings library
81 stars 46 forks source link

bug in merger when old reference results exist #113

Open Mathadon opened 7 years ago

Mathadon commented 7 years ago

I am experiencing problems when merging Annex60 into IDEAS:

1) Consider that a new unit test model is added in Annex60. 2) When merging Annex60 into IDEAS this model and its reference result get merged into IDEAS and the reference result name is copied to .copiedFiles 3) When merging Annex60 the second time the reference result already exists and then the script does not update the results:

new_file = os.path.join(dir_name,
                                                    base_name.replace(self._src_library_name,
                                                                      self._new_library_name))
                            if not os.path.isfile(new_file):
                                copiedFiles.append(new_file)
                                shutil.copy2(srcFil, new_file)

4) since the already existing file is not added to copiedFiles, the file is removed when running

for fil in previouslyCopiedFiles:
            filNam = os.path.join(self._target_home[0: self._target_home.rfind( self._new_library_name )], fil)
            if os.path.isfile(filNam):
                os.remove(filNam)

recall that previouslyCopiedFiles contains all filenames that are in .copiedFiles.txt and not in copiedFiles.

So the result is that reference result files are removed when merging Annex 60.

Proposed solution: In my opinion reference results should be updated when merging Annex60, i.e. the existing file should be updated. I don't see the point of keeping the old reference results? I'll make a pull request.

mwetter commented 7 years ago

@Mathadon If the target library (such as IDEAS) already has a reference result file, then the file from Annex60 should not be copied. The reason is that not all libraries update to the latest Dymola version at the same time. Updating dymola can cause changes to the reference trajectories. In this case, all this changes will be overwritten whenever Annex60 is merged.

Because of the bug you describe above, the files should still be in the copiedFiles.txt as they were originally copied from Annex60.

Mathadon commented 7 years ago

after discussing through voip: We need to fix the underlying issue, for instance by putting the reference result path in copiedFiles even if the result is not overwritten. I could add an option to allow this overwrite anyway for IDEAS.

thorade commented 7 years ago

I think I am having the same issue, or the one described in #66 I can re-run the merger script many times, and every second time the results are copied or not (second run currently has 354 changed/added/deleted files). I think re-running the merger script multiple times should produce identical output and therefore not show any changes in git.

As we are not yet using the reference results in BuildingSystems, I am just ignoring this.

In my opinion, we can also assume everybody HAS to use the same version of Dymola, and just specify which version was used. Then whoever merges Annex60 into another library has to make sure that library uses the same Dymola version for creating the reference results.