Repository: scikit-learn/scikit-learn
File: sklearn/discriminant_analysis.py (Python)

Code before:

            Shrinkage parameter is ignored if  `covariance_estimator` is not None

        covariance_estimator : estimator, default=None
            If not None, `covariance_estimator` is used to estimate the
            covariance matrices instead of relying the empirical covariance
            estimator (with potential shrinkage). The object should have a fit
            method and a ``covariance_`` attribute like the estimators in
            sklearn.covariance. If None the shrinkage parameter drives the estimate.
        """
        n_samples, n_features = X.shape

        means = []
        cov = []
        scalings = []
        rotations = []
        for idx, group in enumerate(self.classes_):
            Xg = X[y == group, :]
            meang = Xg.mean(0)
            means.append(meang)
            if len(Xg) == 1:
                raise ValueError(
                    "y has only 1 sample in class %s, covariance is ill defined."
                    % str(self.classes_[idx])
                )
            cov.append(_cov(Xg, shrinkage, covariance_estimator))
            S2, V = linalg.eigh(cov[-1])
            V = V[:, np.argsort(S2)[::-1]]  # sort eigenvectors
            S2 = S2[np.argsort(S2)[::-1]]  # sort eigenvalues
            Vt = V.T
            rank = np.sum(S2 > self.tol)
            if rank < n_features:
                warnings.warn(
                    f"The covariance matrix of class {group} is not full rank. "
                    "Increasing the value of parameter `shrinkage` might help"
                    " reducing the collinearity.",
                    linalg.LinAlgWarning,
                )
            scalings.append(S2)
            rotations.append(Vt.T)
        if self.store_covariance:
            self.covariance_ = cov
        self.means_ = np.asarray(means)
        self.scalings_ = scalings
        self.rotations_ = rotations

    def _solve_svd(self, X, y):
        """SVD solver for Quadratic Discriminant Analysis.

        Parameters
        ----------

Review comment (refactor):
This method and _solve_svd seem to have the same outer loop (over the classes). Maybe combine them and only have different functions to extract V, S2 and cov from Xg?

Code after:
        scaling, rotation = linalg.eigh(cov)  # scalings are eigenvalues
        rotation = rotation[:, np.argsort(scaling)[::-1]]  # sort eigenvectors
        scaling = scaling[np.argsort(scaling)[::-1]]  # sort eigenvalues
        return scaling, rotation, cov

    def _solve_svd(self, X):
        """SVD solver for Quadratic Discriminant Analysis.

        Parameters
        ----------
        X : array-like of shape (n_samples, n_features)
            Training data.
        """
        n_samples, n_features = X.shape

        mean = X.mean(0)
        Xc = X - mean
        # Xc = U * S * V.T
        _, S, Vt = np.linalg.svd(Xc, full_matrices=False)
        scaling = (S**2) / (n_samples - 1)  # scalings are squared singular values
        scaling = ((1 - self.reg_param) * scaling) + self.reg_param
        rotation = Vt.T

        cov = None
        if self.store_covariance:
            # cov = V * (S^2 / (n-1)) * V.T
            cov = scaling * Vt.T @ Vt

        return scaling, rotation, cov

    @_fit_context(prefer_skip_nested_validation=True)
    def fit(self, X, y):
        """Fit the model according to the given training data and parameters.

        .. versionchanged:: 0.19
            ``store_covariances`` has been moved to main constructor as
            ``store_covariance``.

        .. versionchanged:: 0.19
            ``tol`` has been moved to main constructor.

        Parameters
        ----------
        X : array-like of shape (n_samples, n_features)
            Training vector, where `n_samples` is the number of samples and
            `n_features` is the number of features.

        y : array-like of shape (n_samples,)
            Target values (integers).

        Returns