-
Notifications
You must be signed in to change notification settings - Fork 350
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
Budget Allocator - Date Min/Max not working for some media variables #366
Comments
Hi @Tsurty thanks for reporting this. Actually, the length of the input for |
Hey @laresbernardo I'm pretty sure I had tried that yesterday already and didn't work.
As you can see I'm trying to build it over the robyn_object only as in the meantime I had other stuff running. |
Ok I tried restarting Session, cleaning variables and repeat the process and here's the same error I used to get yesterday
|
Hi Cristian. |
Yes, that's correct. Can you please share your |
I actually have another thing running in the backgroung and it'll take a few hours still so I can't really share with you that one. Anyway, by looking at the code, since I get the error on line 1031 it means that I get past the 1020 -> 1028 lines. |
Nevermind I confused |
Could you try that change on a new branch and test it with your own data to see if your problem gets fixed? If it does, you are more than welcome to submit a PR so we can merge your fix with master. If not, we can keep brainstorming possible sources for this particular issue. I'd like to understand why this is happening to you and have not occurred to other users or on demo.R. Thanks! |
Ok so just to be sure you mean changing the Like I said I'm running another project right now and I'll test the budget allocator on this one too once done, maybe it will work on this one. |
Hello @laresbernardo I finally found some time to test this out but I didn't manage to fix it By printing out the values in the This is what I get in the console: There's a NaN in between the values that may be causing the issue. I've tested this with a bunch of different models but none worked and even tried to use the date_min/date_max functionality with the exact same timeframe as the Rolling Windows but that didn't work either. I'm not really sure what's causing the issue but if you have any feedbacks or tests to try out let me know :) |
Hi Cristian. Do you have a minimum reproducible example you could share? Can you try changing in line 1043, |
Please, share your MRE with me here. |
@laresbernardo Sent it, let me know if you received correctly the .RDS file since it was to big to load I linked it through the Drive but I had no visual feedbacks whether it was actually selected or not |
Note: For the sake of timings, I reduced to 2 trials, which took about 30mins to run. Note: When running
Selected model "1_304_13", which had 2 coef == 0 channels. When running the "max_historical_response" scenario, it ran OK, but effectively there was an error in the message displayed. When running the "max_response_expected_spend" scenario, it also ran OK (once I fixed the dimensions on channel_constr_low and channel_constr_up from your script). When running the "QA optimal response" section, I got his error, which makes sense: " 'media_metric' must be one value from paid_media_spends, paid_media_vars or organic_vars". I noticed that the issue comes from Thanks for the feedback! |
Alright so if I'm understanding correctly the issue is that at least one of organic_vars or paid_media_vars must be positive, even if paid_media_spends are actually positive. I'm traveling today so I won't have time to test this out but I will tomorrow. Thanks a lot in the meantime! |
By definition, we can't have non-positive values for these curves. And the error occurs when you pass negative or not valid numerical values for your |
Hello @laresbernardo I've built another model, completely different data etc. Anyway if I try to run the Budget Allocators, even without any warning about 0 coefs, the date min and max won't work
If I run the allocator without those parameters it works.
An as you can see it returns me an error |
Don't mind about the last error, it was my fault not defining the metric_value variable I've fixed that, tested again and all medias return TRUE with matching values like: I'm not fully getting how this QA section helps, so any clarifications on this would be great. |
This section of the demo.R file gives you an example of how you can calculate and plot the response for a given media variable (paid or not paid). In |
Hi @Tsurty can you please make sure you're testing the latest version? Can you try narrowing it down to a minimum reproducible example? The |
I've just updated the (internal) |
Please try setting this first |
Thanks @laresbernardo this did the trick.
Is it correct that the checks here are done over |
Yes. Please, try changing that check to the following to show both values and make it easier to debug:
|
@laresbernardo The problem was that one of my media channel happens to have all 0 spends in the timeframe used within date min/max, because we chose to turn that off completely. The check you suggested above didn't work because, when the spends are all 0, metric_value doesn't contain 0 but NaN as you can see down here In order to print them all I simply commented the last
I then tried to change my date_min/max to something where all medias had at least some spends and that finally work. I'm glad we managed to understand the issue, however I'd still consider this kind of a bug. What do you think? |
Yes, that's it! I think you've finally found the bug here. So I'll add a new |
@Tsurty I was able to replicate your issue and test my fix. Now we 1. warn the user when there's no spend in a paid media variable (and skip it) and 2. added as skipped media variable in the Can you please update the package and test it out once again? Thanks for catching this case/bug. |
@laresbernardo Yes! It's working correctly now. I'll test this feature more "in depth" in the upcoming days and I'll report if any new issue comes up. Thanks a lot for the help :) |
I'm having this same issue: Running version 3.7.1 (not sure how since latest documented release appears to be 3.7.0...), and I'm seeing very recent updates to |
Hi @JessicaMDuncan The demo.R file has the 3.7.1 and it's currently the latest version. As for your issue, I think that's not a bug but rather the result of |
@laresbernardo that was the error produced at the end of
|
To help me debug this case, can you please share with me your dataset and model's JSON file to see if I can replicate this error? Thanks! |
@laresbernardo I am awaiting confirmation from my leadership team that they will authorize sharing of the data. In the meantime, using placeholder (non-zero spend) data for this channel, I'm now receiving the error discussed in this thread which appears to still be active/ awaiting an update. |
Closing given the inactivity. Feel free to open in case this issue persists. |
Project Robyn
Describe issue
I've this project with Rolling Windows going from 2021-05-25 to 2022-02-28
During Christmas 2021 there has been a huge expense in influencers which is not commonly there, however by running the Budget Allocator we get something like this
For this reason we've been trying to test out the new feature where you set
date_min
anddate_max
in the allocator as follows:The console outputs:
I've tested changing the number of provided boundaries, removing the "date_max" which is simply the end of the dataset but nothing changes.
Environment & Robyn version
The text was updated successfully, but these errors were encountered: