Skip to content
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

Replace fooof with specparam #335

Open
toni-neurosc opened this issue Jun 14, 2024 · 2 comments
Open

Replace fooof with specparam #335

toni-neurosc opened this issue Jun 14, 2024 · 2 comments

Comments

@toni-neurosc
Copy link
Collaborator

toni-neurosc commented Jun 14, 2024

The people that maintain the fooof package have renamed it to specparam and are in the process of releasing a new version.

If we don't need the functionality, we should not have a reason to change, BUT, there is a probem: they forgot a random print in their code which is super-annoying when fooof feature is activated since it prints to the console

            for bwl in self.peak_width_limits:
                print(bwl)

So it might be worth to update just to get rid of that.

On the brighter side, thanks to this I realized that the FOOOF model is initialized once per batch, which is probably not necessary, just initialize once and call fit once per batch.

Reference to make the change to the new version: fooof-tools/fooof#205

EDIT: Ok so I realized that the print is not there anymore, maybe because I re-built my environment and it's possible that it was myself who put the print there months ago doing some research into the FOOOF feature. I feel like an idiot.

@timonmerk
Copy link
Contributor

Good catch with the renaming. Is the print statement still in the specparam package. If so, we should let them know. Tom is also super friendly, we had him once in our lab meeting :)

Yes.. and it would definitely makes sense to drop the reinitialization. I think I looked into that at one point, and in order to prevent updating a forked version of fooof/specparam, I decided to keep it like that.. But maybe the syntax changed now and it's possible to use repetitively an already initialized object.

@toni-neurosc
Copy link
Collaborator Author

Hi @timonmerk , I was working on the FOOOF change and I noticed sth: Numpy is raising a lot of RuntimeWarnings due to this line:

params = params ** (1 / fm.get_params("aperiodic_params", "exponent"))

This is because this is equivalent of trying to take the root of a negative number, which would produce an imaginary number and instead produces nan but gives the warning as well, Because don't understand FOOOF very well, I checked the docs and I think this exponent corresponds to this explanation here: https://fooof-tools.github.io/fooof/auto_tutorials/plot_05-AperiodicFitting.html#a-note-on-interpreting-the-knee-parameter

image

But I don't understand the significance of the knee parameter being negative, my intuition says, seeing as most negative values are very close to zero, that when this happens it means that the data did not have a bend, so there is no knee in this case. Right after the code above there is this line:

features_compute[f_name] = np.nan_to_num(params)

which converts the resulting nan values to 0, which means that the feature output for the knee_frequency feat will be 0 for negative values of k. If I'm correct, then I suggest changing that for:

                        # If knee parameter is negative, set knee frequency to 0
                        if params < 0:
                            params = 0
                        else:
                            params = params ** (1 / fm.get_params("aperiodic_params", "exponent"))

Won't throw warnings and makes the calculation a bit more explicit I think. Also I tested that the warnings happened with the real experimental data by loading the example dataset ses-EphysMedOff throught the nm_IO module.

There was also another problem arising with my artifical random dataset: when the exponent $\chi$ is not 0 but is nonetheless super-small, then the exponent explodes in size and that results in giant outliers in the output, messing the z-score normalization due to float overflows. I was messing with the aperiodic fit formula a bit here: https://www.desmos.com/calculator/dsniwuiyaz?lang=es

And setting a $\chi$ parameter of 0 basically flattens the data to a horizontal line, which I recon is probably impossible in real world data so I won't address that.

However, in the real dataset a new problem appeared: I'm getting a lot of RuntimeWarning: invalid value encountered in log10 with FOOOF due to very negative values of the k parameter resulting in the calculation of negative logs. I'm not sure what this means, if you set k to negative in the little graph I made you get like some sort of inverse exponential curve... Is it possible that either FOOOF is not a good metric for this data, or that the data is already log-normalized or sth?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants