-
Notifications
You must be signed in to change notification settings - Fork 321
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[BUG] Iteration problem when using ensemble SINDy and GenerlizedLibrary #158
Comments
Hi Bartosz, glad you're using all the new functionality. What version of PySINDy are you using? We just corrected an ensembling-related bug from versions 1.5.1 -> 1.6.1 |
Thank you for quick response, I am using the newest version 1.6.1. |
Good, let me try to reproduce this now. |
Small oversight on our part. Looks like when ensemble=True, the library is fit each time, so it adds the tensored libraries the first time and then it is confused when the second call of fit happens. Fixed this and pushing the changes in a moment, so you'll need to get the latest version of the main branch. Let me know if you need anything else, and here is the silly example I used to debug:
|
Hi and thank you for the fast response and fix of the issue, really appreciate it. 👍 |
Hi, sorry for the maybe misguiding answer before, I still appreciate your work ;) but: I tried it now with the new 1.6.2 push but it does not seem to work. I get the same error message. I compared the code for this right now and nothing changed. Is the push already through? Or has something gone wrong? Thanks, bw Bartosz |
Are you cloning the GitHub repo directly or something else? The 1.6.2 push did not have the update... there is another git commit after that, containing the fix. If you are not cloning the repo directly, let me know, and I will just make a v1.6.3 release so you can get it |
I am normally using pip to manage my packages or conda if necessary. I can just git-clone and have it straight away, but maybe it would be good to have it accessible through pip or conda. Thank you for your help :) |
Just published a new release (v1.6.3) with the change. Let me know if that does not fix your issue! |
Thanks for the progress on this, @akaptano ! It looks like the last push and release went part of the way to addressing this issue, but I had to add another line to the generalized library for the weak case (to set the number of samples to the number of domains times the number of trajectories instead of x.shape[0]). I pushed this change to the weak_optimization branch for now. I think some iterations of multiple_trajectories and nonweak/weak libraries may still cause issues, and we probably should do a few tests still before pushing another release. @BMP-TUD, I suggest you clone the github repository and checkout the weak_optimization branch for now. You can use pip to install the developmental version as well, as in:
Since you're using the weak libraries, you will also benefit from faster fitting in the weak_library branch, but you should remove the |
@znicolaou I think you're right but @BMP-TUD if you aren't using the weak form stuff, I think you should be able to get away with using v1.6.3 for now. |
Oops, had a typo in that last push that I fixed, so I pushed again... FYI I am also trying to use the generalized library with multiple trajectories to fit an integro-differential equation with the weak form library, so hopefully I can flesh out all these issues as I go. |
Thank you both, I accessed the package via git clone. It works perfectly. :) |
Addressed this in the new release I believe. Closing this now. |
MTS runs may fail when the train/val/test period (including warmup) at some frequency has a length that is not a multiple of the frequency_factor (factor between lowest and current frequency). This adds a check to find these cases and fail gracefully.
Dear all,
sorry for reaching out to you again, after you already cleared up a few things with the WeakPDELibrary, but I encountered a problem with the Generlized library. Here, I am creating a large library only containing specific terms from a set of 6 different smaller libraries. I have a set of 5 variables, but not all variables are implemented in each Library. This works perfectly fine, with the use of the GeneralizedLibrary and I can fit the model normally. Here is the example on how I create the library (I suppose there is an easier way):
The problem occurs when I try to run the ensemble method together with the generalized library as follows (x_noisy consists of 5 variables):
The error that I get is the following:
The problem here is, that I tried to have a look at what is going on in the code, but changing something in there always breaks the GenerlizedLibrary script. Furthermore, I am confused that this did not happen when I ran the analysis with just the normal SINDy, because the library is the same for both. Maybe you can tell me if I am just doing something wrong, or what the issue could be. Thank you again in advance and I am looking forward to your answer,
Best wishes,
Bartosz
The text was updated successfully, but these errors were encountered: