Closed barrettk closed 2 weeks ago
@seth127 when you get a second, let me know what you think of the compare_nmtran function. I assume we will ditch it, but i'd like to keep that function handy during some development tasks that involve touching control stream files. I suppose we could export it, though I dont think that's too in-line with the original request. It is comparable to the bash script @kyleam was running though.
Edit: Since I removed the compare_nmtran
function, pasting it below for later reference:
This is looking good to me, but I'll leave the final review for @andersone1
@kyleam could you take a look at how we're building the path to the NMTRAN executable and make any comment on whether we think this will work on Windows. And if not... do we know a better pattern for that?
Related, we execute it with processx
. I think this will work, regardless of OS, assuming we have the nmtran_exe
set correctly, but I wanted to double-check that with you too.
@kyleam could you take a look at how we're building the path to the NMTRAN executable and make any comment on whether we think this will work on Windows.
Looking just at those linked lines, nothing jumps out at me that will be an issue.
That's assuming all the processing outside those lines is correct on Windows, but the best option would of course just be to try it out. Before this is merged, I can test the command on our Windows instance for bbr/bbi testing. Or perhaps better: I can get @andersone1 and @barrettk set up to access. Let me know what you all prefer.
Related, we execute it with
processx
. I think this will work, regardless of OS, assuming we have thenmtran_exe
set correctly, but I wanted to double-check that with you too.]
Right. processx in general supports Windows.
I'll pull my comment from https://github.com/metrumresearchgroup/bbr/issues/650#issuecomment-1930725642 for consideration:
Based on how
nmfe*
invokesNMTRAN.exe
...$ grep tr/NMTRAN /opt/NONMEM/nm75/run/nmfe75 # $dir/tr/NMTRAN.exe $prdefault $tprdefault $maxlim < $1 >& FMSG $dir/util/nmtran_presort < tempzzzz1 | $dir/tr/NMTRAN.exe $prdefault $tprdefault $maxlim $do2test >& FMSG $dir/util/nmtran_presort < tempzzzz1 | $dir/tr/NMTRAN.exe $prdefault $tprdefault $maxlim >& FMSG
... I think it's worth considering these questions:
- What does
nmtran_presort
do? Should we be processing the input with that too to follownmfe*
?nmfe*
relays some arguments toNMTRAN.exe
. Is that something that something this check should do too? Are there cases where this new NM-TRAN check would fail but the actual run would pass due to this check not passing the same args to theNMTRAN.exe
call?The answer may be that none of that matters in the context of this check, but that'd still be good to document here.
@barrettk @andersone1 see both comments from @kyleam above. Could the two of you coordinate to do the following:
presort
and flags thing that he mentions in the most recent comment.Thanks all. I'm looking forward to getting this feature out there.
Args
Example
closes https://github.com/metrumresearchgroup/bbr/issues/650