Optional sigmoid transformation and bugfix #44
Conversation
Signed-off-by: bkleyn <[email protected]>
| if contexts is None: | ||
| num_contexts = 1 | ||
| num_contexts = len(contexts) if contexts is not None else 1 | ||
| if num_contexts == 1: |
There was a problem hiding this comment.
makes sense.. so this would make sure the output from predict_expectations is a list
| if transform_scores: | ||
| expectations = expit(pd.DataFrame(expectations)[self.mab.arms].values) | ||
| else: | ||
| expectations = pd.DataFrame(expectations)[self.mab.arms].values |
mab2rec/rec.py
Outdated
| @@ -288,7 +288,7 @@ def predict_expectations(self, contexts: Union[None, List[List[Num]], np.ndarray | |||
| return self.mab.predict_expectations(contexts) | |||
|
|
|||
| def recommend(self, contexts: Union[None, List[List[Num]], np.ndarray, pd.Series, pd.DataFrame] = None, | |||
| excluded_arms: List[List[Arm]] = None, return_scores: bool = False) \ | |||
| excluded_arms: List[List[Arm]] = None, return_scores: bool = False, transform_scores: bool = True) \ | |||
There was a problem hiding this comment.
can you remind me again why this was useful to have this option?
re: naming, given that this is a boolean.. is_sigmoid? has_sigmoid? has_sigmoid_transform? is_sigmoid_applied?
There was a problem hiding this comment.
is this a "breaking" change now, OR we are okay since it has a default setup as True
There was a problem hiding this comment.
No, not breaking. Optional argument and default behavior is the unchanged.
There was a problem hiding this comment.
re: naming - how about "apply_sigmoid"?
There was a problem hiding this comment.
@wddcheng when you get a chance, can you change this to apply_sigmoid, merge the PR, and close the related Issues please?
|
changed transform_scores to apply_sigmoid. all tests passed. Merging the PR now |
No description provided.