asikist / nnc

A framework for neural network control of dynamical systems over graphs.
MIT License
52 stars 11 forks source link

Missing data, wrong import statements #1

Closed celestialteapot closed 2 years ago

celestialteapot commented 2 years ago

Hello,

I found your paper interesting and would like to try out your methods, but I have trouble running your code. Most of the notebooks miss a data folder and many import statements have wrong paths. The Google Collab example suffers from the same problem. Am I missing something?

lubo93 commented 2 years ago

Can you check if the following example runs our your machine: examples/ct_lti_small/small_example.ipynb? I just tested it, and it works. If this example works on your machine too, can you please list the examples that weren't working? Thanks!

celestialteapot commented 2 years ago

Thanks for the quick reply!

This example works mostly, altough the last code cell raises a ValueError: image

One I had an import problem with is examples/kuramoto/complete_graph.ipynb, starting with: image

The first cell runs OK if I add two other paths: image

But then it fails on the second cell, missing a data folder which I couldn't find in the repo manually either: image

asikist commented 2 years ago

Hi @celestialteapot, thanks for sharing. The file small_example.ipynb is now updated after your request in commit 86d063eac5f6dc2f08da0f030cfe381e16224543. As you correctly mention, the suggested paths need to be appended to python path to include the required helpers. We will commit a fix for this later or feel free to submit a pull request. Regarding the data, we did not upload them to Github, as the experiments produced a considerable amount of data. The missing file in that example is an adjacency matrix (N x N) that represents the graph, where the dynamics evolve. You will also need to generate the parameters of the dynamics. You may generate adjacency matrices and parameters of dynamics by running this file. Please make sure to place them in the correct paths or update the paths in the notebook accordingly. If you have trouble generating the parameters, please let us know and we will send you the files directly.

P.S. For feedback control, most of the data can be accessed here: https://ieee-dataport.org/documents/neural-ordinary-differential-equation-control-dynamics-graphs

celestialteapot commented 2 years ago

Hi @asikist, thank you, happy to contribute!

I made a little pull request from the fixed import paths. As for small_example.ipynb, I'm afraid your fix didn't solve the ValueError. The shape of the groupby cumsum is currently (20000, 3), but you want it to be a single column, so I think what you need is simply alldf.groupby(['lr', 'sample_id'])['energy'].cumsum(). I submitted a separate pull request with this modification, but I'm not 100% sure, so please be sure to verify it before accepting.

Concerning the kuramoto examples generating the data with utilities.py happily solved the missing directory problem (but I did need to rename parameters.pt to parameters_225.pt and fix the relative paths). I understand that you generated a lot of data, but just a suggestion, why not add this small data folder to the repo to make it run OK by default?

Lastly, my newest problem is that the NODEC solver doesn't seem to work properly: image image Do you have an idea how to fix this?

asikist commented 2 years ago

Perfect, thank you very much for the pull request and also for providing a clear direction for improvement of the repo. I will review your request and update the repo.

Lastly, my newest problem is that the NODEC solver doesn't seem to work properly:

The attached plot looks like the controller did not update parameters. The attached output indicates that an exception (potentially numerical) occurred and parameters resumed to the best know values (probably initial values). This is one possibility. I also notice the following message "module 'signal' has no attribute ...". I do not remember seeing this message and I am unsure whether this prevents training by causing an exception. Nevertheless, can you try the following: reduce learning rate considerably, e.g. at least an order of magnitude smaller and increase training epochs. Do you still get the instability? Does loss improve over epochs? In the next days we will look into it in detail and prepare a small working example that does not require a lot of data to be uploaded to Github.

celestialteapot commented 2 years ago

That didn't help, it just keeps running into numerical instability and reducing the learning rate but not the loss.

Turns out the cause is indeed the signal module, as signal.SIGALRM and signal.alarm(time) work only on UNIX systems, while I am currently using Windows 11. I found no simple fix (maybe this), other than commenting out the call in neural_net.py, which "solves" the issue: image image

I suppose time_limit is luckily not an essential function; might want to look into cross-platform alternative solutions though.

asikist commented 2 years ago

@celestialteapot , yes I also see this online. We will check this as well in the following days and see if we can replace. Indeed, I would suggest removing the time_limit and signal module calls from the code in windows. Unless you use adjoint_odeint or are trying to solve a numerically challenging case, the non-usage of time_limit is not expected to impact performance.

celestialteapot commented 2 years ago

Alright, thanks!