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

Problem with plot_Hinss2021_classification #683

Open
allwaysFindFood opened this issue Feb 7, 2025 · 2 comments
Open

Problem with plot_Hinss2021_classification #683

allwaysFindFood opened this issue Feb 7, 2025 · 2 comments

Comments

@allwaysFindFood
Copy link

allwaysFindFood commented Feb 7, 2025

In the official example plot_Hinss2021_classification.py (https://neurotechx.github.io/moabb/auto_examples/plot_Hinss2021_classification.html), is there a logical error on line 76?

    """Select channels based on covariance information."""

    def __init__(self, n_chan, cov_est):
        self._chs_idx = None
        self.n_chan = n_chan
        self.cov_est = cov_est

    def fit(self, X, _y=None):
        # Get the covariances of the channels for each epoch.
        covs = Covariances(estimator=self.cov_est).fit_transform(X)
        # Get the average covariance between the channels
        m = np.mean(covs, axis=0)
        n_feats, _ = m.shape
        # Select the `n_chan` channels having the maximum covariances.
        all_max = []
        for i in range(n_feats):
            for j in range(n_feats):
                if len(all_max) <= self.n_chan:
                    all_max.append(m[i, j])
                else:
                    if m[i, j] > max(all_max):    # This is  line 76
                        all_max[np.argmin(all_max)] = m[i, j]
        indices = []
        for v in all_max:
            indices.extend(np.argwhere(m == v).flatten())
        # We will keep only these channels for the transform step.
        indices = np.unique(indices)
        self._chs_idx = indices
        return self

It should be: if m[i, j] is greater than the minimum value, then replace it.


                    if m[i, j] > min(all_max):
                        all_max[np.argmin(all_max)] = m[i, j]

Can the entire code segment for finding the largest n values be optimized to?

np.partition(m.flatten(), -n_feats)[-n_feats:]
@bruAristimunha
Copy link
Collaborator

Hey @gcattan,

Can you help @allwaysFindFood please?

@gcattan
Copy link
Contributor

gcattan commented Feb 7, 2025

Hi @allwaysFindFood
Thanks for suggestions ! Indeed, looks like there is a place for improvement !

How do you feel about raising a PR?

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

3 participants