Skip to content

Fix/mread pow convert#1381

Merged
kylebaron merged 5 commits into
mainfrom
fix/mread-pow-convert
May 1, 2026
Merged

Fix/mread pow convert#1381
kylebaron merged 5 commits into
mainfrom
fix/mread-pow-convert

Conversation

@kylebaron

@kylebaron kylebaron commented Apr 24, 2026

Copy link
Copy Markdown
Collaborator

@kyleam Found the regression below in mapbayr after 2.0.0 release.

The problem comes with the call to convert_pow_spec() which had late move to the main mread() flow, right before writing out the cpp file.

  1. We wanted to pass in the "original" block names to use in warning messages when certain code can't be converted; this came from incoming_block_names, which could longer than the length of spec when the names were captured
  2. We have to convert $MAIN in the mapbayr example, that could be NULL (no code); while convert_pow() can handle NULL, assigning x[[i]] <- NULL blows that position away causing problems when processing subsequent blocks.
  • convert_pow_spec() only checks when length > 0
  • I added this as defensive move in the other convert_*_spec() functions (however, these aren't called in the same context and I don't expect this same issue)
  • convert_pow_spec() doesn't check blocks beyond the length of block_names with the idea that these blocks were added by mread() as shim and don't need to be converted.
> 
> ── Error ('test-read_mrgsolve_model.R:71:3'): adm_0_cmt works ──────────────────
> <subscriptOutOfBoundsError/error/condition>
> Error in `x[[i]]`: subscript out of bounds
> Backtrace:
>>  1. └─mrgsolve::mcode(...) at test-read_mrgsolve_model.R:71:3
>  2.   └─mrgsolve::mread(...) at mrgsolve/R/mcode.R:57:3
>  3.     └─mrgsolve:::convert_pow_spec(spec, mread.env$incoming_names) at mrgsolve/R/mread.R:460:5
>  4.       └─mrgsolve::convert_pow(x[[i]], to_convert_names[i]) at mrgsolve/R/modspec.R:510:5
> ```
> 
> check output
> ```
> ── R CMD check results ───────────────────────────────────── mapbayr 0.10.2 ────
> Duration: 1m 19.3s
> 
>checking tests ...
>   See below...
> 
> ── Test failures ───────────────────────────────────────────────── testthat ────
> 
> > library(testthat)
> > library(mapbayr)
> 
> Attaching package: 'mapbayr'
> 
> The following object is masked from 'package:stats':
> 
>     filter
> 
> > 
> > test_check("mapbayr")
> 
> 
> [ FAIL 1 | WARN 0 | SKIP 4 | PASS 717 ]
> 
> ══ Skipped tests (4) ═══════════════════════════════════════════════════════════
>On CRAN (4): 'test-progress.R:5:3', 'test-reset-conditions.R:3:3',
>   'test-reset-conditions.R:93:3', 'test-reset-conditions.R:175:3'
> 
> ══ Failed tests ════════════════════════════════════════════════════════════════
> ── Error ('test-read_mrgsolve_model.R:71:3'): adm_0_cmt works ──────────────────
> <subscriptOutOfBoundsError/error/condition>
> Error in `x[[i]]`: subscript out of bounds
> Backtrace:
>>  1. └─mrgsolve::mcode(...) at test-read_mrgsolve_model.R:71:3
>  2.   └─mrgsolve::mread(...) at mrgsolve/R/mcode.R:57:3
>  3.     └─mrgsolve:::convert_pow_spec(spec, mread.env$incoming_names) at mrgsolve/R/mread.R:460:5
>  4.       └─mrgsolve::convert_pow(x[[i]], to_convert_names[i]) at mrgsolve/R/modspec.R:510:5
> 
> [ FAIL 1 | WARN 0 | SKIP 4 | PASS 717 ]
> Error: Test failures
> Execution halted
> 
> 1 error| 0 warnings| 0 notes> ```
> 
> https://github.com/FelicienLL/mapbayr/blob/3df5f6afeed675bc751038b2071e730109cbdf64/tests/testthat/test-read_mrgsolve_model.R#L71-L74
> 
> ```
>   #$MAIN but empty
>   mod0bis <- mcode("mod0bis",
>                 "$CMT GUT CENT
>                 $MAIN
>                 ", compile = FALSE)
> ```
> 
> I'm hoping we can let this through as "not a functional issue".
> 
> The other option is to pin mrgsolve and wait for an update on either the mrgsolve or mapbayr side.
> 
> cc: @kyleb

@FelicienLL

Copy link
Copy Markdown
Collaborator

Hi Kyle, do not hesitate to let me know if there is anything you would like me to test or check or update on my end.
Kind regardd

@kylebaron

Copy link
Copy Markdown
Collaborator Author

Hey thanks @FelicienLL . No update needed; this is 100% on us. mapbayr had an edge test case that revealed an oversight in some new code. It should be addressed now; will reach out if anything else come up.

Kyle

@kylebaron kylebaron requested a review from kyleam April 25, 2026 22:29
@kylebaron kylebaron merged commit 535dbfa into main May 1, 2026
5 checks passed
@kylebaron kylebaron deleted the fix/mread-pow-convert branch May 1, 2026 12:15
@kylebaron kylebaron mentioned this pull request May 19, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants