Example suggestion

[edumagol]

Hi guys!

We would like to contribute sharing an example of the use of PySindy in a real case scencario where we modeled a SISO (single input - single output) process control loop with a PID controller.

We believe that our example could help many others which intend to use Sindy do discover industrial process dynamics. ]

Please let me know if you would like us to share the code with you and also how we should do it.

All the best, Edu

[akaptano]

We would be quite happy to have it! Feel free to paste in the code or some details into this issue. If there is a paper, please link that as well. You can also send me the full notebook at [email protected] and I can take a look at + clean it up for addition as a formal example if it looks like a nice contribution that showcases the SISO use case.

[edumagol]

Hi @akaptano !

Thanks! I will anonymize data first and than prepare the jupyter notebook (we developed the example using *.py scripts at first hand). I will bring news soon.

All the best and many thanks for your interest @akaptano

[Jacob-Stevens-Haas]

Hey @edumagol, if it's already in .py and you're converting to .ipynb, one request I have for new notebooks is factoring the data payload to allow testing with smaller data. We've had issues with refactoring because tests don't cover every case in the notebooks, but the notebooks take a long time to run (see #203).

I believe Project Jupyter has given some thought to this and probably has a better answer, but one pattern that works is the following project structure:

./project/
  | - example.ipynb  # calls `from example_data import gen_data1, gen_data2...` in first cell.
  | - example_data.py  # definition of `gen_data1()`, `gen_data2()`...
  | - mock_data.py  # alternate definitions of `gen_data1()`, `gen_data2()`... with smaller data payloads

In an actual notebook cell, you can have

x, u, t = gen_data1()

That way, a test runner could load example.ipynb as a python object, replace the import statement in memory, and verify example.ipynb runs with a much quicker test. People actually opening example.ipynb and running it, however, will get the notebook on the full data.

[edumagol]

Hi @Jacob-Stevens-Haas !

Sorry for my delayed response... we are preparing the code according to your guidelines. Hope we will have it finished soon and will let you know!

All the best Edu

[edumagol]

Hi guys @akaptano and @Jacob-Stevens-Haas , good news!

I believe the example is ready, so that you can check if the code meet your requirements (I hope, so!). If not, please just let me know.

I uploaded all the files in this repo: https://github.com/ihmstefanini/dynamic-modeling-pysindy

I still have to upload the data file, which is a csv, but it is too large (> 25MB). I will probably compress it and will fine tune the data_processing.py code in order to be able to read data from a different format (now it reads * csv format).

In mean time, you can start to check if there is any inconsistence in the rest of the code/jupyter notebook file...

Hope our contribution can help other in the community.

Thanks for this amazing repo: PySindy!!!

P.S. - We are also finishing a multi input-output example, where we also use the model to run a python implementation of a MPC. Let me know if you are also interested in this new example. If so, we will prepare it for the community too.

Thanks and keep in touch, Edu

[edumagol]

Hi @Jacob-Stevens-Haas and @akaptano ,

Just added the data as a parquet file and now it is all set, I mean, the example is all functional and you should be able to reproduce it.

Hope we can hear from you soon.

All the best,

[Jacob-Stevens-Haas]

Hey, sorry for the delay, and thanks for your patience. I've run the file, and it runs locally well. From a purely techincal standpoint, I added some guidance for examples in #203. Basically, I think we should require that:

1. Save the notebook as a python file also. Running tests on python files requires a lot less overhead on than running tests on jupyter notebooks and produces better debug outputs.
2. Change the notebook name to example.ipynb and the python file to example.py. This is so that our test runner knows where to find it. You can always sync example.ipynb $\leftrightarrow$ example.py using the jupytext package (it's what we're starting to do).
3. The ONLY local imports allowed: example_data, mock_data, utils. Local imports are given a name in the global sys.modules namespace, and so if we're running tests on multiple notebooks, those names can conflict. We remove example_data, mock_data, utils before and after each notebook test. Any other local module, if you need one, must be a dotted submodule of these names, e.g. utils.plotting. Currently the sys.path setup/teardown won't work for something like utils.plotting but ~I'll PR a fix~ #227.
4. I'm going to ask that the notebook tests in under ten seconds. This took 113 seconds. You can add smaller mock data to your repo for faster tests. The trick we developed sets the notebook's/script's __name__ = "testing" when testing the notebook (at other times, __name__ is set by the python interpreter, usually "__main__"). You can check this variable in the notebook to choose whether to import your real data or the mock data. See how we did it to reduce the run time of notebook 5 from 20 min to 10 sec.

Sorry if that's a lot - the point of this is to allow us, as we change pysindy in potentially non-backwards-compatible ways, to quickly check all examples to see if anything breaks. Otherwise, we've seen its fairly easy to break examples unexpectedly.

I'm ~going to add a PR~ added a PR with the test I wrote to handle 3rd party examples, so you can try it on your notebook. It'll also clarify the guidance on 3rd party examples and have a section to link to your (and other 3rd party) examples.

Separate issue: I'm not sure what the right way to handle examples that require additional dependencies. For the moment, @edumagol don't worry about this. @akaptano , @znicolaou , current thoughts are to ignore example dependencies and require tester to notice errors/know to install them, like in example 9. It's a problem for the future.

[edumagol]

No problem @Jacob-Stevens-Haas ! We will take a look at all of your comments and make all the necessary changes. Thanks and talk to you soon!

[akaptano]

@edumagol Any update on this?