feat: RSSD penalty for 0 media effects media (parameter) #680
Conversation
This file contains 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
enabling penalisation of model decomp with 0 media effects.
facebook-github-bot
added
the
CLA Signed
laresbernardo
changed the title
laresbernardo
commented
Apr 11, 2023
R/R/model.R
Outdated
| @@ -42,6 +42,8 @@ | |||
| #' "unconstrained". By default, if intercept is negative, Robyn will drop intercept | |||
| #' and refit the model. Consider changing intercept_sign to "unconstrained" when | |||
| #' there are \code{context_vars} with large positive values. | |||
| #' @param rssd_0eff Logical. When TRUE, the objective function DECOMP.RSSD will | |||
There was a problem hiding this comment.
- I'd change
rssd_0effwith a less complex parameter name, maybe something likerssd_penaltyorrssd_zero_effector ... something without digits - We use "Boolean" instead of "Logical" across the documentation.
- Shouldn't this parameter live in the error calculation rather than here? We can pass the parameter via ... instead.
- Should we be more explicit on how we penalize?
- Should we apply this on
robyn_refresh()as well? - This is a quite relevant change - we should adapt the documentation on the site as well to reflect the change.
CC @gufengzhou
There was a problem hiding this comment.
- then rssd_zero_penalty
- ok
- No, it's part of the objective function, not just a score for output. or where do you suggest?
- It's actually quite clear IMO. Given the same RSSD score, a model with 50% 0-coef will get penalized by RSSD * 1.5 (larger error), while another model with no 0-coef gets unpenalized with RSSD *1
- For refresh, I'm not sure at the moment, because the RSSD for refresh is calculated from the net new refresh datapoints, with the purpose to have beta coefs reflecting bigger spend changes --> If I stop spending on channel A for a week, ideally this should be reflected or not penalized. I'm not sure about this yet.
- Yes we should!
There was a problem hiding this comment.
Ok, pending only:
- Update site with this new parameter information (even though I've added your example to the internal parameter documentation).
- Agree on refresh default value.
QQ: why would a user set rssd_zero_penalty = FALSE? Is it necesary we enable it as a parameter?
laresbernardo
changed the title
Closed
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.
No description provided.