Avoid storing any unnecessary variable after fit

[wdevazelhes]

Some algorithms currently store a reference to the data at fit time (having a self.X_ attribute for instance). According to scikit-learn guidelines (https://scikit-learn.org/stable/developers/contributing.html#fitting), we should avoid this:

Note that the model is fitted using X and y, but the object holds no reference to X and y. There are, however, some exceptions to this, as in the case of precomputed kernels where this data must be stored for use by the predict method.

If we really need all the data we could maybe store a copy of X but I don't think it is the case in metric-learn's algorithms

[bellet]

Which algorithms do this? I agree that it should not generally be needed.

[wdevazelhes]

Currently in the PR preprocessor #117 it was present in LFDA, LMNN, and MLKR, but I changed it in commit wdevazelhes@f2d0cd7 (and the following commit) so when the PR is merged this issue should be closed

[bellet]

So once the API branch is merged we can close this as well?

[wdevazelhes]

Yes !

[wdevazelhes]

I just updated the title, see comment #139 (comment)

[wdevazelhes]

Here is a list of all variables that are stored at fit time. We should decide whether we keep them because they are useful for the user after the estimator is fitted (like components_ in a PCA for instance), (and in this case we should add them in the docstring under the Attributes title), or make them temporary objects. For each, in comment I'll put a guess for the previous answer, but feel free to fill it if you can.

List of attributes (except transformer)

• ITML:

  • bounds_: ?
  • A_: should be discarded since we have transformer_

• LMNN:

  • labels_inds: probably should be discarded but to check
  • n_iter_: should be kept and added to "Attributes"

• LSML:

  • vab_: ?
  • vcd_: ?
  • w_: ? they are weights, todo: check in scikit-learn if they store it
  • prior_inv_: discard ?
  • M_: discrad since there is already transformer_
  • n_iter_: keep and add to Attributes

• MLKR:

  • n_iter_: keep and add to Attributes

• MMC:

  • A_: remove (already transformer_)
  • n_iter_: keep and advertise

• NCA:

• n_iter_: idem

• SDML:

  • M_: remove (already transformer_)

[wdevazelhes]

We need to fix this before fixing #142

[bellet]

ITML: bounds_ can be kept

LMNN: label_inds should be discarded (related to labels in training set)

LSML: vab_, vcd_ should be discarded (related to training quadruplets), prior_inv_ also (simply inverse of prior), w_ indeed you can check in sklearn (these are importance weights on the training data, not sure whether we have them for this approach and not for others)

[wdevazelhes]

Thanks @bellet ! I just made a PR to solve the issue: #159