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

[BUG] MASE transform and fit separation #978

Open
dokato opened this issue Jul 29, 2022 · 2 comments
Open

[BUG] MASE transform and fit separation #978

dokato opened this issue Jul 29, 2022 · 2 comments
Labels
bug Something isn't working

Comments

@dokato
Copy link
Contributor

dokato commented Jul 29, 2022

Expected Behavior

I can call fit and then transform on MASE object.

Actual Behavior

MASE works when I use the fit_transform method. When I try to generate separate embeddings for my individual graphs it fails with a quite not intuitive error.

Example Code

# this works
Vhat = embedder.fit_transform(adj_list)
# this doesn't
embedder = MASE(n_components=5)
embedder.fit(adj_list)
embedder.transform(adj_list[0])

Full Traceback

---------------------------------------------------------------------------
NotFittedError                            Traceback (most recent call last)
/var/folders/zt/s4rb4_kj4pnbmcqw7xyw52hw0000gn/T/ipykernel_19051/4212279286.py in <module>
----> 1 embedder.transform(pass_to_ranks(conn_mats[0]))

~/projects/graspologic/graspologic/embed/base.py in transform(self, X)
    305 
    306         # checks
--> 307         check_is_fitted(self, "is_fitted_")
    308         if isinstance(X, nx.classes.graph.Graph):
    309             X = import_graph(X)

~/anaconda3/envs/grasp2/lib/python3.9/site-packages/sklearn/utils/validation.py in check_is_fitted(estimator, attributes, msg, all_or_any)
   1343 
   1344     if not fitted:
-> 1345         raise NotFittedError(msg % {"name": type(estimator).__name__})
   1346 
   1347 

NotFittedError: This MultipleASE instance is not fitted yet. Call 'fit' with appropriate arguments before using this estimator.

At first I thought that MASE fit method just lacks

self.is_fitted_ = True

line. But then I noticed that whole _compute_oos_prediction abstract method is not implemented here.
Isn't that just as simple as in ASE? I.e.:

return X @ Vhat

Not sure @bdpedigo if this is what you referred to in our email conversation as a "tricky part"?

Your Environment

  • Python version: 3.9.7
  • graspologic version: 1.0.0
@dokato dokato added the bug Something isn't working label Jul 29, 2022
@bdpedigo
Copy link
Collaborator

you are correct, because of some changes to the base class a separate fit/transform was added when it should not have been (see #797)

a transform (for new graph(s)) could absolutely be implemented. I can show you how to do it given a network and the V matrices (or it may be mentioned in the original paper) if having a transform is important for your application. And we could totally add it to graspologic as well, but we'd just need to sketch out an API for it.

@dokato
Copy link
Contributor Author

dokato commented Jul 31, 2022

Thanks Ben, I can help with adding that. What original paper are you referring to? I guess not this one? The scaffolding of API looks good to me, the only thing to figure out is what to returns in case of single vs multiple graph input to the transform method.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants