-
Notifications
You must be signed in to change notification settings - Fork 20
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
[WIP] Domain-invariant partial least squares regression #286
base: main
Are you sure you want to change the base?
Conversation
Thanks for the PR! It seems your linter changed |
Hi Antoine I have reverted the changes as you suggested but it seems that the tests are still not successful... |
Hello @B-Analytics , You need to merge the README.md file because the current merge conflit prevent te doc and the test from running. |
Got it! |
Hey guys I still face problems with the tests. However, this time the problem is associated with the docs. Following files seem to cause the problem: ../../examples/deep/plot_adversarial.py Can you please give me a hand? |
You can generate and open the doc locally with |
Which version of scikit-learn are you using? |
Yes, I am using scikit-learn v1.5.2 on my local machine. Let me know if I can do something to move this forward. |
Skorch’s new version is coming soon: skorch-dev/skorch#1085. It should fix the CI! |
The scikit-learn team fixed the problem; let's check if the tests pass now! |
Hello @B-Analytics! Now that the tests are passing, can we review or do you still need some time to finish them? |
@tgnassou, yes please go ahead with the review. Thx! |
on the source and target domain, the DIPLS and JDOT Regressor on the same task and | ||
illustrate the learned (domain-invariant) features. | ||
|
||
.. [36] Nikzad-Langerodi, R., Zellinger, W., Saminger-Platz, S., & Moser, B. A. (2020). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's 37 now
@@ -61,6 +61,11 @@ | |||
OTLabelProp, | |||
JCPOTLabelPropAdapter, | |||
JCPOTLabelProp) | |||
|
|||
from ._dipls import ( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
no need to put it in two lines
---------- | ||
xs : ndarray of shape (n_source_samples, n_features) | ||
Feature data from the source domain. | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
don't put space between lines of params
|
||
References | ||
---------- | ||
Ramin Nikzad-Langerodi et al., "Domain-Invariant Regression |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
missing ref number
|
||
Parameters | ||
---------- | ||
xs : ndarray of shape (n_source_samples, n_features) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
try to keep the convention of skada using Xs
instead of xs
# Preliminaries | ||
self.n_, self.n_features_in_ = Xs.shape | ||
self.ns_, _ = Xs.shape | ||
self.x_ = Xs |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't get why you use x
and xs
while there are the same data?
I made a little review but more to format the code writing. I'll read more in-depth soon. However, your PR lacks tests for your method to check if everything's working fine. Just get inspired by the already existing tests for shallow methods. |
Summary
I added the
DIPLS
class along with thedipals
function toskada/_dipls.py
and created an example inexamples/methods/plot_dipls.py
to showcase its usage. In the example, I apply di-PLS to a typical regression task in my domain (analytical chemistry) and tried to highlight the features that make it particularly useful for us (interpretability, flexibility). I also included a comparison with JDOT on the same task. The branch passed all tests (related to the new class and functions) and I updated the documentation accordingly.