search

scikit-adaptation / skada

Domain adaptation toolbox compatible with scikit-learn and pytorch
https://scikit-adaptation.github.io/
BSD 3-Clause "New" or "Revised" License
52 stars 16 forks source link

[MRG] Fix issue of the circular validation with deep models #169

Closed YanisLalou closed 2 months ago

YanisLalou commented 2 months ago

Fixes #163 #139

codecov[bot] commented 2 months ago

Codecov Report

Merging #169 (9531a4a) into main (625b22a) will decrease coverage by 13.93%. The diff coverage is 44.44%.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #169 +/- ## =========================================== - Coverage 97.42% 83.50% -13.93% =========================================== Files 48 41 -7 Lines 4624 4346 -278 =========================================== - Hits 4505 3629 -876 - Misses 119 717 +598 ```
kachayev commented 2 months ago

In light of skorch Net not supporting deep copy, there are 2 possible scenarios:

  1. It's just an oversight on their end. Would be nice to open issue or PR for them to explain the need, maybe they would just add deepcopy on their side (so we will be able to remove workaround on our side).

  2. There's a deeper non obvious issue that prevents Net objects from being "deep-copyable" in the first place (which could easily happen because Torch is mostly in C and C objects/initializers are not always done friendly for performing copy from Python side). It would be absolutely necessary for us to understand those limitations.

rflamary commented 2 months ago

We need a working CircCV and we cannot wait fro skorch developers butw e shoudl definitely open an issue on skorch to ask about your point 1.