Skip to content
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

Failed to generate one pagers when ts_validation is enabled in 3.9.0 #596

Closed
yanhong-zhao-ef opened this issue Jan 6, 2023 · 7 comments
Closed
Assignees
Labels
bug Something isn't working

Comments

@yanhong-zhao-ef
Copy link

yanhong-zhao-ef commented Jan 6, 2023

Project Robyn

Describe issue

When enabling ts_validation for OutputModels, robyn_outputs doesn't output the one-pager plots in the plot folder with the following error in the stack trace

>> Exporting general plots into directory...
trace: robyn_plots
Failed exporting results, but returned model results anyways:
 Error in ts_validation(OutputModels, quiet = TRUE, ...): object 'OutputModels' not found

This is due to the lines in robyn_plots (shown below). robyn_plots is called by robyn_outputs where OutputModels is not explicitly supplied as an argument to robyn_plots:

  OutputModels$ts_validation_plot <- ts_validation(OutputModels, quiet = TRUE, ...)
  if (!is.null(OutputModels$ts_validation_plot)) {
    ggsave(
      paste0(OutputCollect$plot_folder, "ts_validation", ".png"),
      plot = OutputModels$ts_validation_plot, dpi = 300,
      width = 10, height = 12, limitsize = FALSE
    )
  }

I tried modifying the lines above to read from OutputCollect$OutputModels instead of OutputModels but OutputCollect$OutputModels was modified earlier in the robyn_outputs function, therefore, doesn't contain resultCollect anymore to extract from for the ts_validation function.
The quick fix where I changed the function signature of robyn_plot to function (InputCollect, OutputCollect, OutputModels, export = TRUE, ...) and called it in robyn_outputs with all_plots <- robyn_plots(InputCollect, OutputCollect, OutputModels, export = export) worked out fine.

Running through demo.R as it is doesn't show this bug but we are orchestrating Robyn in another script like source(robyn_func.R) where we wrap robyn_run and robyn_outputs in functions.

Please see attached demo_debug.txt to see a reproducible example where I just wrap those two functions calls (robyn_run and robyn_outputs) into a function and the bug reveals.

Provide reproducible example

demo_debug.txt

Environment & Robyn version

Make sure you're using the latest Robyn version before you post an issue.

  • Check and share Robyn version: packageVersion("Robyn") ‘3.9.0’
  • R version (Please, check and share: sessionInfo() or R.version$version.string) R version 4.2.2 (2022-10-31)
@yanhong-zhao-ef
Copy link
Author

Morning team, just checking back in, is a fix almost there? :)

@michellegrushkometa
Copy link
Contributor

hi @yanhong-zhao-ef thank you for raising this issue and providing your code and example! Working on a fix this week. Stay tuned!

@breogancid
Copy link

breogancid commented Jan 17, 2023

Hi alll,

@yanhong-zhao-ef, you can use <<- to assing the OutputModels meanwhile they can solve the issue:
OutputModels <<- robyn_run(...)

@michellegrushkometa, maybe you can check the plot.R file.

In the line 225, the code fails because OutputModels is not defined (It isn't a function parameter). You can access to than with OutputCollect$OutputModels.

The code:

robyn_plots <- function(InputCollect, OutputCollect, export = TRUE, ...) { ...

    OutputModels$ts_validation_plot <- ts_validation(OutputModels, quiet = TRUE, ...)
       if (!is.null(OutputModels$ts_validation_plot)) {
        ggsave(
          paste0(OutputCollect$plot_folder, "ts_validation", ".png"),
          plot = OutputModels$ts_validation_plot, dpi = 300, width = 10, height = 12, limitsize = FALSE
    )
  }

@yanhong-zhao-ef
Copy link
Author

Hi alll,

@yanhong-zhao-ef, you can use <<- to assing the OutputModels meanwhile they can solve the issue: OutputModels <<- robyn_run(...)

@michellegrushkometa, maybe you can check the plot.R file.

In the line 225, the code fails because OutputModels is not defined (It isn't a function parameter). You can access to than with OutputCollect$OutputModels.

The code:

robyn_plots <- function(InputCollect, OutputCollect, export = TRUE, ...) { ...

    OutputModels$ts_validation_plot <- ts_validation(OutputModels, quiet = TRUE, ...)
       if (!is.null(OutputModels$ts_validation_plot)) {
        ggsave(
          paste0(OutputCollect$plot_folder, "ts_validation", ".png"),
          plot = OutputModels$ts_validation_plot, dpi = 300, width = 10, height = 12, limitsize = FALSE
    )
  }

Thanks @breogancid I can patch up the code locally but I would rather get the official release from the team. Btw I did try that one OutputCollect$OutputModels but then this object is modified in a previous step so a naive fix didn't work for me.

@laresbernardo
Copy link
Collaborator

The bug was found and will be patched up later today. In short, OutputCollect$OutputModels (from robyn_outputs()) and OutputModels (from robyn_run()) are different and were being treated as equal. Fix on the way.

@laresbernardo
Copy link
Collaborator

laresbernardo commented Jan 20, 2023

Hi @yanhong-zhao-ef, @breogancid thanks for your patience! I've just landed the fix for this issue. We tested for ts_validation TRUE and FALSE, both behaving as expected. Can you please confirm? Feel free to close this issue if everything's running smoothly now.

@yanhong-zhao-ef
Copy link
Author

Thank you for the fix and can confirm that the whole thing runs now!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

4 participants