cmu-delphi / flu-hosp-forecast

1 stars 0 forks source link

Use `rpy2` in runner #14

Closed dshemetov closed 1 year ago

dshemetov commented 1 year ago

Using rpy2 to call R lets us:

Hopefully this isn't too installation specific and can work well with renv.

brookslogan commented 1 year ago

issue (blocking): using rpy2 does appear to be part of the cause of long/infinite run times. With OPENBLAS-OPENMP enabled, I don't see any additional cores launch. It's paused on an empty progress bar, so I wonder if that's the culprit (and if your evalcast update solves it when parallelism isn't used or progress bars are disabled).

brookslogan commented 1 year ago

Some observations:

dshemetov commented 1 year ago

Linking this related issue rpy2/rpy2#961.

brookslogan commented 1 year ago

Still testing to try to determine what's actually going on.

Buggy debugging: - ~Trying only running the second model: instead it aborts! But the error message seems to be truncated, complicating debugging; not sure truncation is from rpy2 or something else. [Something else! This seems to match the non-rpy2 output. But I don't understand why my current modifications are generating errors for the second model.] [I think this might be something related to evalcast/rlang/other library versions with/without my renv.]~ --- No, I just tried passing a list to `get_predictions`' forecaster arg instead of a function and was confused by errors.
brookslogan commented 1 year ago

I noted some of our debugging findings here. I would suggest we don't move forward with rpy2 for now, and instead: either check the wait/exit status from os.system, or move to something using subprocess as recommended in linked doc. From an earlier experience: we should also check that we properly raise an error within R when install.packages encounters an issue; I think it might only warn in some cases when it should stop.

dshemetov commented 1 year ago

Closing in favor of #17