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

Return data from the backend fit method #835

Merged
merged 3 commits into from
Jul 13, 2023

Conversation

mauicv
Copy link
Collaborator

@mauicv mauicv commented Jul 12, 2023

What is this:

this fixes: #813. Given we fit differently in different cases the data returned is different in case.

  • SVM(..., optimization='sgd'): Returns dictionary containing converged and n_iter where n_iter is the number of iterations until convergence.
  • SVM(..., optimization='bgd'): Returns dictionary containing converged, n_iter and lower_bound where n_iter is as above and lower_bound is the loss achieved during training.
  • GMM(..., backend='pytorch'): converged, lower_bound and n_epochs - because the GMM PyTorch backend performs gradient descent over n epochs this value is the number of epochs performed.
  • GMM(..., backend='sklearn'): converged, lower_bound and n_iter.

@mauicv mauicv requested a review from jklaise July 12, 2023 16:15
Copy link
Member

@jklaise jklaise left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM but would add the info on what's returned in the public docstrings of fit. Also seems fit_logs isn't used?

Is the plan also to return info from the existing ODs? As a minimum we should open a ticket and investigate what those returns might be.

@codecov
Copy link

codecov bot commented Jul 12, 2023

Codecov Report

Merging #835 (05ed926) into master (2a0c090) will increase coverage by 0.01%.
The diff coverage is 100.00%.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #835      +/-   ##
==========================================
+ Coverage   81.86%   81.87%   +0.01%     
==========================================
  Files         159      159              
  Lines       10317    10323       +6     
==========================================
+ Hits         8446     8452       +6     
  Misses       1871     1871              
Impacted Files Coverage Δ
alibi_detect/od/pytorch/gmm.py 93.75% <ø> (ø)
alibi_detect/od/pytorch/svm.py 99.16% <ø> (ø)
alibi_detect/od/_gmm.py 97.43% <100.00%> (ø)
alibi_detect/od/_svm.py 97.50% <100.00%> (ø)
alibi_detect/od/pytorch/base.py 97.43% <100.00%> (+0.21%) ⬆️

@mauicv
Copy link
Collaborator Author

mauicv commented Jul 13, 2023

LGTM but would add the info on what's returned in the public docstrings of fit. Also seems fit_logs isn't used?

Is the plan also to return info from the existing ODs? As a minimum we should open a ticket and investigate what those returns might be.

Agree w.r.t. the docstrings, good catch! I'll add that update. I'll open a ticket for the old ODs. They could definitely return something but it's a bit of a bigger job. ticket here: #837

@mauicv mauicv merged commit c580354 into SeldonIO:master Jul 13, 2023
11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Return data from od fit methods
2 participants