test-copy-model-from.R: adjust check for bbr change

[kyleam]

copy_model_as_nmbayes() calls bbr::copy_model_from() underneath. copy_model_from() is supposed to overwrite the $PROBLEM record for the copied model, but it only handled upper cases record names until bbr's 13fb8e5f (copy_control_stream: use nmrec instead of regex, 2023-07-21).

A copy_model_as_nmbayes() test checks for lines that it expects to be in the parent model but not the new one. It uses "$prob" for the record name, and expects the old bbr behavior (i.e. the same problem line is in both the parent and child), leading to a failure with the unreleased bbr.

Use an upper case $PROB in the parent model so that copy_model_from() updates the record for bbr's versions before and after 13fb8e5f.

Re: https://github.com/metrumresearchgroup/bbr/pull/604

---

Test matrix

bbr.bayes main, bbr 1.7.0

$ git rev-parse HEAD
2894a2f7e0d674f18d9e4ca39d9dd87025fbde1f
$ git -C ../bbr rev-parse HEAD
d12884cb7611dbd8ac21904fc0931f0ff707642e

$ Rscript -e 'devtools::test(filter = "copy-model-from")'
ℹ Testing bbr.bayes
✔ | F W  S  OK | Context
✔ |         52 | copy-model-from [1.1s]

══ Results ════════════════════════════════════════════════════════════════════════════════════════════════════════════
Duration: 1.1 s

[ FAIL 0 | WARN 0 | SKIP 0 | PASS 52 ]

bbr.bayes this PR, bbr 1.7.0

$ git rev-parse HEAD
2323f5a196dc9109b9e7f19dac3d8cff1c199a77
$ git -C ../bbr rev-parse HEAD
d12884cb7611dbd8ac21904fc0931f0ff707642e

$ Rscript -e 'devtools::test(filter = "copy-model-from")'
ℹ Testing bbr.bayes
✔ | F W  S  OK | Context
✔ |         52 | copy-model-from [1.1s]

══ Results ════════════════════════════════════════════════════════════════════════════════════════════════════════════
Duration: 1.1 s

[ FAIL 0 | WARN 0 | SKIP 0 | PASS 52 ]

bbr.bayes main, bbr main

$ git rev-parse HEAD
2894a2f7e0d674f18d9e4ca39d9dd87025fbde1f
$ git -C ../bbr rev-parse HEAD
e5546f08055e181728944d365f8543ca55d3ab6d

$ Rscript -e 'devtools::test(filter = "copy-model-from")'
ℹ Testing bbr.bayes
✔ | F W  S  OK | Context
✖ | 1       51 | copy-model-from [1.1s]
───────────────────────────────────────────────────────────────────────────────────────────────────────────────────────
Failure ('test-copy-model-from.R:110:3'): copy_model_as_nmbayes() keeps trailing comments
setdiff(lines_parent, mod_lines) not identical to c("$EST METHOD=SAEM", "  abort", "$EST METHOD=ITS").
Lengths differ: 4 is not 3
───────────────────────────────────────────────────────────────────────────────────────────────────────────────────────

══ Results ════════════════════════════════════════════════════════════════════════════════════════════════════════════
Duration: 1.1 s

── Failed tests ───────────────────────────────────────────────────────────────────────────────────────────────────────
Failure ('test-copy-model-from.R:110:3'): copy_model_as_nmbayes() keeps trailing comments
setdiff(lines_parent, mod_lines) not identical to c("$EST METHOD=SAEM", "  abort", "$EST METHOD=ITS").
Lengths differ: 4 is not 3

[ FAIL 1 | WARN 0 | SKIP 0 | PASS 51 ]

bbr.bayes PR, bbr main

$ git rev-parse HEAD
2323f5a196dc9109b9e7f19dac3d8cff1c199a77
$ git -C ../bbr rev-parse HEAD
e5546f08055e181728944d365f8543ca55d3ab6d

$ Rscript -e 'devtools::test(filter = "copy-model-from")'
ℹ Testing bbr.bayes
✔ | F W  S  OK | Context
✔ |         52 | copy-model-from [1.1s]

══ Results ════════════════════════════════════════════════════════════════════════════════════════════════════════════
Duration: 1.1 s

[ FAIL 0 | WARN 0 | SKIP 0 | PASS 52 ]

[kyleam]

@barrettk there's no rush on this, but when you have a moment, would you mind taking a look?

[kyleam]

@seth127 Adding you too, if you happen to have a moment before @barrettk does.

[kyleam]

I've pushed an update to make Drone also test against bbr's main to help catch issues like this.

If that commit is picked on top of this repo's current main, the build fails as expected: https://github-drone.metrumrg.com/metrumresearchgroup/bbr.bayes/388

[barrettk]

I've pushed an update to make Drone also test against bbr's main to help catch issues like this.

Yeah I think this is a good idea. Does it make any sense to do this with nmrec in bbr as well you think? I imagine that package wont change as frequently (in general), so probably not worth the extra CI time, but just putting it out there.

[barrettk]

@kyleam looks like my approval isn't worth much until im added as a collaborator:)

[kyleam]

looks like my approval isn't worth much until im added as a collaborator:)

Don't listen to GitHub. It's still worth something to me :)

(added you)

[kyleam]

Does it make any sense to do this with nmrec in bbr as well you think? I imagine that package wont change as frequently (in general), so probably not worth the extra CI time, but just putting it out there.

I agree nmrec is probably less likely to break things (partly due to more focused domain, partly because bbr.bayes is reaching into the guts of bbr a lot more), so in my opinion it's not worth a dedicated pipeline for it. However, I wouldn't be against sneaking it into the cran-latest pipeline (letting the latest release on MPN be tested by mpn:latest).

(Similarly for testing nmrec's main here we could piggyback off the just-added bbr-main pipeline.)

[seth127]

Good discussion here. I'll defer to y'all on the details and just say that I'm glad we're thinking about these interdependencies.