watertap-org / watertap-ui

User interface(s) (UI) for the WaterTAP library
MIT License
4 stars 13 forks source link

Add test for parameter sweep #93

Open adam-a-a opened 10 months ago

adam-a-a commented 10 months ago

image

Currently, if the developer does something "wrong" such as naming a flowsheet file differently than the ui export filename, parameter sweep in the UI will fail, showing the above error message. However, if the developer never tried running the sweep tool while tinkering with the UI, this would be an unknown issue and slip through the cracks.

While I know there aren't any tests in place yet for parameter sweep, rather than propose comprehensive testing, I think we should start with at least a test that (1) runs parameter sweep across some arbitrary variable on a flowsheet to make sure no exception is raised or (2) check file naming to make sure parameter sweep would run if invoked in the UI; we can assert that naming is in expected format (i.e., UI code filename matches flowsheet filename within same directory).

MichaelPesce commented 9 months ago

It might make sense to address the source of this issue directly rather than creating a test to enforce the rule.

The error occurs because of how we currently import the solve function from flowsheets. On the UI backend, we have access to the flowsheet export module, but not the flowsheet module itself. I was attempting to dynamically import the original flowsheet by taking the flowsheet export module and removing the “_ui” part of the file name. So if a flowsheet export file doesn't follow the "\<flowsheet-name>_ui.py" naming convention, an error is thrown.

I was under the impression that it was necessary to send the original solve function as a parameter in order to run the sweep (ie, the solve function inside the flowsheet module). However, it appears that I can use the flowsheet export’s solve function instead, so this dynamic import of the original flowsheet is unnecessary.

Rather than enforcing the naming convention by adding a test, I suggest that I make the change in the parameter sweep file to use the flowsheet export's solve function, removing the import of the original flowsheet.