Default settings for non-MLE PCA raise warning

[tsalo]

Summary

The default settings for sklearn's PCA (used for the Kundu tedpca decision tree, but not MLE) involve deriving as many components as there are volumes in the time series. The last component explains no variance and causes the following warning to be raised for both the three-echo rest and five-echo task test datasets:

tedana/tedana/model/fit.py:169: RuntimeWarning: divide by zero encountered in true_divide
  F_S0 = (alpha - SSE_S0) * (n_echos - 1) / (SSE_S0)

Additional Detail

From what I recall, PCA should estimate n_vols - 1 components, not n_vols, which would explain why the last component explains no variance and is causing this divide-by-zero warning. I think we should override the default number of components here with n_vols - 1. This will prevent the warning, but the components that matter will be the same.

However, I also noticed that changing the number of components will also impacts the decision tree, so I decided to raise an issue where we could discuss this rather that opening a small bug-fix PR.

Next Steps

1. If we decide to override the default number of PCA components, then here:
   

   tedana/tedana/decomposition/eigendecomp.py

   Lines 301 to 306 in 82447c5

   	ppca = PCA()
   	ppca.fit(dz)
   	comp_ts = ppca.components_
   	varex = ppca.explained_variance_
   	voxel_comp_weights = np.dot(np.dot(dz, comp_ts.T),
   	np.diag(1. / varex))

   
   we can simply change ppca = PCA() to ppca = PCA(n_components=(n_vols - 1))

[jbteves]

Pending the complexity of the decision tree, which I have yet to fully wrap my head around, I would think that we should just completely remove anything with a variance of zero. If we trust our variance metric, then variance 0 components don't matter. I could see this being a problem, though, if the number of components from PCA is somehow used in the decision tree (though I don't think it is from what I've read/seen in the code).

[tsalo]

One of our three measures of variance explained (now called original normalized variance explained) comes pretty much directly from the PCA, and that one's zero for this component as well, so I think we can trust the metric.

Classifications aren't directly dependent on the number of components (afaik), but including extra components does affect the classification because of the elbows.

[emdupre]

I suppose this is waiting on team-decomp ?

[eurunuela]

I’d say it’s save to fix the number of components to be n_vols - 1.

@tsalo , how does changing the number of components impact the decision tree?

[tsalo]

As far as I can recall, it doesn't impact the results much, since it only changes the number of components by one.

[eurunuela]

Then I’d fix it and close the issue. Does that sound right to you @tsalo ?

[tsalo]

So... it looks like I already addressed this in #364 and forgot to tag this issue in the PR. Sorry about that! Closing now.