asikist/nnc

Missing data, wrong import statements

Closed this issue · 8 comments

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?

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!

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

Hi @celestialteapot, thanks for sharing.
The file small_example.ipynb is now updated after your request in commit 86d063e.
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

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?

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.

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.

@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.

Alright, thanks!