Breaking change somewhere between v0.0.3 and f4089a6

[papsti]

I've found a breaking change somewhere between the release of v0.0.3 and the latest version on main (f4089a6). This is not super urgent on my end as I can just work with v0.0.3 for now. This issue may very well just stem from me using old macpan2 functionality that isn't supported past v0.0.3, but that would be problematic given that there hasn't been a formal version update.

A reprex can be seen with EPACmodel. Quick warning that running this whole example will take a few mins as it requires two full macpan2 installs...

# fresh start
remove.packages("oor")
remove.packages("macpan2")
remove.packages("EPACmodel")

# restart R

# install working version of EPACmodel
remotes::install_github("canmod/oor") # needed because this version of EPAC model relies on a version of `macpan2` where `oor` had to be installed manually
remotes::install_github("phac-nml-phrsd/EPACmodel")

This version of EPACmodel depends on macpan2 v0.0.3 and the following runs without issue:

EPACmodel::make_simulator(model.name = "five-year-age-groups")

Output here is

Classes 'TMBSimulator', 'TMBSimulationFormatter', 'Base' <environment: 0x0000023ab27fddf0>             
ad_fun : function ()  
error_code : function (...)  
gradient : function (...)  
hessian : function (...)  
matrix : function (..., matrix_name, time_step)  
matrix_names : function ()  
objective : function (...)  
report : function (..., .phases = c("before", "during", "after"))  
simulate : function (..., .phases = c("before", "during", "after")) 

which is expected.

However, if you instead install the bugged version of EPACmodel that depends on f4089a6 (which I wanted to do to get around the manual install of oor):

# fresh start
remove.packages("oor")
remove.packages("macpan2")
remove.packages("EPACmodel")

# restart R

# install bugged version of EPACmodel
remotes::install_github("phac-nml-phrsd/EPACmodel@bugged")

and again run

EPACmodel::make_simulator(model.name = "five-year-age-groups")

I get the following error:

Error in as.matrix(flow)[settings$flow(), , drop = FALSE] :                                            
  subscript out of bounds

To be clear, there is absolutely no difference between the two versions of EPACmodel except the macpan2 version listed in DESCRIPTION.

[stevencarlislewalker]

Thanks for this issue.

I will quickly note that back-compatibility is only monitored between tagged versions. All other versions should be considered development versions and so it is always possible that it hasn't been fully tested.

I'll look more into this issue.

[stevencarlislewalker]

Hmmm ... I'm on a completely busted experimental branch of macpan2 and your example just seems to work. I'll try being more careful with your example to see if I can reproduce.

[stevencarlislewalker]

I've reproduced.

[stevencarlislewalker]

The flows component of the default values object is missing all of the infection flows. Yes they should be NA, but they should also be present. This is the same issue that I fixed with my recent pull request to the EPAC repo. This suggests that v0.0.3 was more robust.

[stevencarlislewalker]

I'm thinking that this is the change:

[papsti]

The flows component of the default values object is missing all of the infection flows. Yes they should be NA, but they should also be present. This is the same issue that I fixed with my recent pull request to the EPAC repo. This suggests that v0.0.3 was more robust.

I actually remembered the empty_matrix -> NA fix you did before, so at one point I changed all the N.xx entries in the model$simulators$tmb()call to NA (in models/five-year-age-groups/simulator-expression.R), in case that was the issue. Doing that got me different error:

matrix information passed to c++ is not valid
Error in self$check(x):
    Error in valid$check(x): mats component is not of type numeric

This may just be because I don't really understand exactly when one should use NA and when one should use macpan2::empty_matrix. In the case above, I left the initial values for the other matrices computed via derivations.json (whose names ended with .) as macpan2::empty_matrix.

[papsti]

The flows component of the default values object is missing all of the infection flows. Yes they should be NA, but they should also be present. This is the same issue that I fixed with my recent pull request to the EPAC repo. This suggests that v0.0.3 was more robust.

Oh, OK, I understand what you're saying. Do you want me to try to update default_values.rds to include all flows, and then see if that fixes things?

[stevencarlislewalker]

Sorry @papsti . I was just documenting my progress. No need for you to try anything yet.

[stevencarlislewalker]

Hi @papsti. 98fb990 is intended to fix this issue by making this component more robust. In particular, it should allow both NA and completely missing flows (and states for that matter). All tests are passing as well. I'm tempted to bump to version 0.0.4, but I'd like to at least get a thumbs up from you that nothing else is broken when using the current main branch.

[papsti]

wonderful, i'll check it out now and let you know

[papsti]

bumped the macpan version specified in the EPACmodel DESCRIPTION file on the bugged branch to the patch commit and reinstalled EPACmodel from there, ran the package tests, and compiled the README.Rmd, double-checking that the patched version of macpan2 was being installed with EPACmodel.

previously when using f4089a6, my package tests would fail and the README.Rmd file wouldn't compile. now everything has worked, and i've reproduced this on two systems, so it's good on my end! thanks for a quick fix 😄

[papsti]

please let me know if you bump to v0.0.4, so that i can update the DESCRIPTION file on EPACmodel to that instead of a commit number.