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

Implement sample weights in Quadratic class #245

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

hoodaty
Copy link

@hoodaty hoodaty commented Apr 10, 2024

Context of the PR

To try and account for the weights for samples, I have tried to modify the Quadratic class, since it was an issue during MCPRegression where the sample had weights.

Contributions of the PR

Tried to appropriately multiply the weights wherever required.

Checks before merging PR

  • added documentation for any new feature
  • added unittests
  • edited the what's new(if applicable)

@hoodaty hoodaty changed the title Nbb Add ElasticNet like regularization for SparseLogisiticRegression Apr 10, 2024
@hoodaty hoodaty changed the title Add ElasticNet like regularization for SparseLogisiticRegression Altered Quadratic Class Apr 10, 2024
@mathurinm
Copy link
Collaborator

@hoodaty can you also edit the first message of this PR to link to the related issue?

You need to add a unit test for this, again a comparison against sklearn's Lasso with sample_weights seems the easiest way in my opinion.

@QB3
Copy link
Collaborator

QB3 commented Apr 12, 2024

Quick design question:

since almost no code is shared in the altered functions, would it be worth it to add a class WeightedQuadratic inheriting from Quadratic, and overwriting only the functions we need?

@QB3 QB3 mentioned this pull request May 10, 2024
@mathurinm mathurinm changed the title Altered Quadratic Class Implement weights in Quadratic Class May 14, 2024
@mathurinm mathurinm changed the title Implement weights in Quadratic Class Implement sample weights in Quadratic class May 14, 2024
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.

None yet

3 participants