allow user to modify model prediction results#12
Open
mikoontz wants to merge 3 commits into
Open
Conversation
…utputs from predict_learner() output. This gives the user more flexibility over the information being used to ultimately derive CPI.
…no errors, warnings, or notes
…operly load when using ?cpi
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR addresses some of the need that arose in #10. In some use cases (not sure how common!), a user may need to modify the
truth,response, orprobvalues output generated by thepredict_learner()function. This patch introduces two new arguments to thecpi()function (which get passed to thecompute_loss()function):modify_trpand....modify_trp("trp" stands for truth/response/prob) is by defaultFALSE(leave thetruth/response/proboutput alone) but can also take a user-defined function that accepts as arguments the originaltruth,response, andprobvalues (after the modification that takes place in the beginning of thecompute_loss()function) as well as the...argument to allow for some flexibility. It must return a named list (names must betruth,response, andprob) with the updated values. If values don't need updating, a user can simply pass the original value to the corresponding list item in the returned object.Some potential use cases (which I've added examples for in the docs):
probobject from theresponseobject, and to update theresponseobject to represent the predicted class.It strikes me that I can imagine the
modify_trpand...arguments being passed to thepredict_learner()function instead ofcompute_loss(), but the machinery at the start ofcompute_loss()makes thetruth,response, andprobobjects a little easier to work with (because they're made consistent regardless of whetherinherits(pred, "Prediction")is TRUE or FALSE). But this block could also move to the end ofpredict_learner()to make thepredoutput consistent at this stage?Just a thought on organization (keeping all the prediction modification within
predict_learner()instead of spread acrosspredict_learner()andcompute_loss()) to discard if not useful!I think this approach allows maximum flexibility for a user, but I'm also happy to contribute to further discussion on other ways to implement something like this (if it might be implemented at all).
All checks pass and I added some worked examples to the documentation.
Thanks again for the great package!