KindXiaoming / pykan

Kolmogorov Arnold Networks
MIT License
13.63k stars 1.2k forks source link

The purpose of KAN class and pykan is unclear. #28

Open hrshtt opened 2 months ago

hrshtt commented 2 months ago

The KAN class is an implementation of an nn.Module, classically in ML, this expects it to be modular, reusable, extensible, etc. yet there are many utility methods that try to make it something more. The whole implementation of KAN is clearly more in the application side trying to make it easier for people to interface with KANs (most likely targeted at scientists and letting them do quick experiments). However for ML use-cases, it would be more beneficial to give a clean and modular implementation of KANs, which leave more room for pytorch like plug-n-play usage.

Given this dichotomy of expectations from ML RnD vs Scientific RnD, would it not be more feasible to have a minimal implementation of the KAN class which leans more towards existing torch/ML expectations and then have a wrapper library that uses the KAN class for further scientific usage?

KindXiaoming commented 2 months ago

Hi, thank you for the comment! Making a good sense to me! Yeah KANs, as they are now, are designed mostly to aim scientists make discoveries (that was my initial motivation). The accuracy and parameter efficiency parts are also trying to convince people to use it for scientific tasks (which are relatively low-dimensional and potentially have compositional structures). To be honest, I did not expect the ML world to act this strongly to this so did not make an attempt to make it efficient and extensible etc. I'd welcome the community to build their own projects to improve KANs efficiency/extensibility, since my personal focus lean more towards AI for science. This repo will be maintained as targeting users who are more interested in making scientific discoveries with KANs as assistants or collaborators. Thank you for noticing our work, and thank you for bringing this issue up! I really appreciate it!

hrshtt commented 2 months ago

That is understandable, I appreciate you having written this repo and replying to my issue. I am learning a lot by playing around with KANs at the moment.

However, I would still suggest to think of a way to isolate the low-level KAN modules with the high-level experimentation with KAN methods. Having these separated will let the community better help with maintenance and optimization. While also keeping focus on the scientific utility clear. This would potentially be a win-win for either groups, as a sensible underlying structure can be constantly improved while keeping it compatible with the scientific experiments API.

imo it would be beneficial to do this in the current repo, as having a new source of truth for such technical work would be really hard to maintain. I can see you have gone through a lot of trouble to make the code so clean and with such extensive examples, i would be more than glad to help out with such an interesting project!

KindXiaoming commented 2 months ago

Hi, I'm glad that you are willing to contribute to this project! In that case I'm convinced that we should separate the low-level KAN modules and the high-level experimentation, as long as both groups (science researchers and Machine learning researchers) are happy to use. Thank you for your help in advance!

hrshtt commented 2 months ago

That sounds great! I'll come up with a proposal for some changes. Then you can assign this issue if you want. For now i've been compartmentalizing the KAN class into:

modeling could have a lot of plug-n-play. splines, RBFs (in a future), etc symbolic could contain all the utilities needed for experiments, and potentially does not care about how the data was modeled hence keeping it separate visualization could contain the plotting pieces, and could be called directly by symbolic or other modules.

KindXiaoming commented 2 months ago

Great, the three part compartmentation sounds good to me!

hrshtt commented 2 months ago

update: I have the directory structure ready, i have made sure that the from kan import * usage across all the notebooks still works as expected. kan/modeling/numeric_KAN.py contains NumericKAN class which contains all the methods other than the symbolic ones, kan/symobolic/KAN.py contains class KAN(NumericKAN) which is equal to the original kan/KAN.py KAN class as well as imports kan/visualization/visualize.py plot method (the usage of the prune method requires plot method to be part of the KAN class itself)

./kan
├── __init__.py
├── modeling
│   ├── LBFGS.py
│   ├── __init__.py
│   ├── numeric_KAN.py
│   ├── numeric_KANLayer.py
│   └── spline.py
├── symbolic
│   ├── KAN.py
│   ├── __init__.py
│   └── symbolic_KANLayer.py
├── utils.py
└── visualization
    ├── __init__.py
    ├── figures
    └── visualize.py

5 directories, 12 files

I have tested the code on a few notebooks, will do further testing.

I am also not separating the train method from NumericKAN class for now, so as it keep the focus on the new directory structure.

KindXiaoming commented 2 months ago

my notebooks contain a lot of model.plot() and model.train() as well, please make sure they also run properly. Thanks in advance!

viktor-ktorvi commented 2 months ago

It'd be really good to set up a good amount of tests beforehand. For example, replicating the results from the paper but unittestable, e.g., making sure a symbolic expression is identified correctly, or that a certain RMSE level is reached for a fixed set of hyperparameters. You don't want to refactor blind and question if everything works the same as before.

KindXiaoming commented 2 months ago

Hi @hrshtt , thank you for all your efforts! I'm recently quite swamped and will need some time to deal with other stuff before checking your PR. Also I know I might be asking too much, but could you please also try run examples in tutorials to see if their results preserve? My experience is that they could be quite sensitive to seed or code reorgnaization. If that's too much trouble for you, may I propose another possibility? You might consider creating separate repo under your account which I'll also refer to as a more reusable/modular version in my readme. I understand the "source of truth", but merging two and doing double check might be too much work both for me and for you. The field is fast pacing, ML researchers will create their repos for efficient/modular KANs (like you!) anyway, and I may still want to host this repo as a home for scientific computing/discoveries. I may take your advice and do some code reconstruction later, but I tend to do it myself in a slow and cautious way. Thank you for your understanding!

hrshtt commented 2 months ago

Hi, you raise a valid point. I have been testing the code constantly while doing my changes. I have ran almost every notebook in kan/docs/Examples and kan/docs/API_demo. The solution works on all of them and I will raise an issue where the solution does not run due to errors in the notebook itself.

HOWEVER, when you say reproducibility of results, I am already in the process of documenting the issue, that the library does not produce reproducible results.

Here I ran Example_5_special_functions on a fresh installation of pykan from pypi on colab. The results are not the same as they are in the notebook present in the repository.

It might be fair to hold off major changes until results are reproducible across notebooks (unless i am doing something wrong here, or not understanding the intent of the seed value)

image

My suspicion is maybe the notebooks in the repo may be from an older run after which the code has already changed and the notebooks not ran again.

hrshtt commented 2 months ago

i feel contributions of large scope would be premature at this stage, given the state of reproducibility. ill maintain my own fork for now, hope you can get more hands on this project, its great work!