verivital / hyst

HyST: A Source Transformation and Translation Tool for Hybrid Automaton Models
http://verivital.com/hyst/
Other
15 stars 18 forks source link

Continuization Pass with args4j #9

Closed stanleybak closed 8 years ago

stanleybak commented 8 years ago

I converted continuization pass to use args4j instead of commons cli. I also added two unit tests for the continuization pass, and confirmed that they work after the changes were made.

Please check the changes and let me know if you approve, Taylor.

ttj commented 8 years ago

Some tests are failing due to a lack of scipy. I think it was set up before so these could either throw an error to pass if no scipy, or succeed with correct values if installed. I updated so it would pass on my system.

I looked at the python tests to see how it's done there.

As a better fix, given we need to detect what's installed in several places, should we refactor it somewhere in the main to check if scipy is installed, if so set a flag, and if that flag is false, just not run the tests, etc.? This would also give the ability to know what features are available, and I think will start to show up more (e.g., we could do a test for Stateflow, we could do a test for Z3, we could do a test to see if the analysis tools [SpaceEx, etc.] are installed, etc.).

ttj commented 8 years ago

Sorry, I think I pushed my updates to the wrong branch (master instead of this), didn't realize I needed to push to this (and using GUI so it defaulted to master). I was able to successfully test this in 2015b, 2015a, 2014b, and 2014a after making a minor change (which I now need to commit) to the stateflow converter (just adding the jar to the java path).

stanleybak commented 8 years ago

Hmm, you're right the hybridization pass tests would assume that scipy is installed since they used this for optimization.

I'll add some code to detect if the feature is available, as you suggested. I don't like having all the tests that use scipy done in one junit test, as python-tests is doing, anyway.

stanleybak commented 8 years ago

Okay, I changed the way pythonbridge works and added a "static boolean hasPython()" which can be used to detect if python is present and has all the libraries without raising exceptions. I updated PythonTests and PassTests to be parameterized so that they execute twice, once with python, and once faking that python doesn't exist on the system. This should prevent the error you saw Taylor where some tests were failing, even though they all passed on my system.

I also merged in the changes from the master branch that you pushed.

Let me know if this is all set now, and I'll merge the pull request.

ttj commented 8 years ago

Ok, that sounds generally okay for now, although I think have more fine-grained failing would ideal (e.g., fail if some library is not there), especially if additional stuff is planned to be added in Python. E.g., I think this approach now prevents me from running regression tests, since that leverages python?

I just tested and all tests pass and it seems to still work fine in Matlab (only tested 2015b), so I think it's okay to merge.

stanleybak commented 8 years ago

Hmm, I think right now the regression tests only convert the models without any passes and check if the tools seem to run them for 5 seconds or so without giving an error (basically check if the tools accept the models output by Hyst). The tests for the passes are done as unit tests, in PassTests.java. So I think both the unit tests and regression tests should work for you.