Skip to content

Conversation

@allglc
Copy link
Collaborator

@allglc allglc commented Sep 29, 2025

Context

  • Add the possibility of multi risk control
  • Keep unidimensional lambda (using thresholding on predict_proba)

Content

This PR includes (lists incrementally updated):

  • Update typing of BinaryClassificationController

Copy link
Collaborator

@Valentin-Laurent Valentin-Laurent left a comment

Choose a reason for hiding this comment

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

Good start 💪🏻

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR adds support for multi-risk control to the BinaryClassificationController by updating type annotations and adding validation logic. It allows users to specify multiple risks and corresponding target levels while maintaining backward compatibility with single risk control.

  • Updated type hints to accept lists of risks and target levels alongside single values
  • Added validation to ensure risks and target levels lists have matching lengths
  • Enhanced class documentation to describe multi-risk control functionality

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

@allglc allglc force-pushed the binary-risk-control-multi-risk branch from d0b64b6 to cb272b1 Compare October 8, 2025 13:19
Copy link
Collaborator

@Valentin-Laurent Valentin-Laurent left a comment

Choose a reason for hiding this comment

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

Reviewing the __init__ method and submethods.

Nice job!

Regarding typing, we could use a slightly stricter MyPy flag to forbid untyped functions (right now, only partially untyped function are forbidden). But if I remember, using that flag would cause a lot of typing to do in the existing codebase (not 100% sure though). So right now, let's try to not forget typing new code.

cc @GBrelurut

Copy link
Collaborator

@Valentin-Laurent Valentin-Laurent left a comment

Choose a reason for hiding this comment

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

Reviewing method calibrate (except ltt_procedure).

@allglc
Copy link
Collaborator Author

allglc commented Oct 17, 2025

My latest commit updates ltt_procedure for multi risk. Calls to the function and tests have been adapted. Input types and/or shapes have been modified.

I also replaced np.where by np.nonzero, which is recommended by numpy doc in the case when only the condition is provided.

Copy link
Collaborator

@GBrelurut GBrelurut left a comment

Choose a reason for hiding this comment

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

Ok for me.
Nice PR.

@GBrelurut GBrelurut marked this pull request as ready for review October 21, 2025 15:23
@GBrelurut GBrelurut merged commit dfc95ae into master Oct 21, 2025
6 checks passed
@GBrelurut GBrelurut deleted the binary-risk-control-multi-risk branch October 21, 2025 15:31
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.

4 participants