metrumresearchgroup / bbi

Next generation modeling platform
11 stars 2 forks source link

params is incompatible with NONMEM 7.3 ext output #321

Open kyleam opened 1 month ago

kyleam commented 1 month ago

NONMEM 7.3 doesn't write the special -1000000007 or -1000000008 iterations to the ext. When -1000000007 isn't present, bbi nonmem params fails with an index error:

dir,error,termination,THETA1,THETA2,THETA3,THETA4,THETA5,SIGMA(1_1),OMEGA(1_1),OMEGA(2_1),OMEGA(2_2)
panic: runtime error: index out of range [0] with length 0

goroutine 1 [running]:
github.com/metrumresearchgroup/bbi/cmd.params(0xc0000f9400?, {0xc0000be8a0, 0x0, 0x7a0d5c?})
    /data/home/kylem/src/github/metrumresearchgroup/bbi/cmd/params.go:392 +0x1a39
github.com/spf13/cobra.(*Command).execute(0xc000113608, {0xc0000be880, 0x2, 0x2})
    /data/home/kylem/go/pkg/mod/github.com/spf13/cobra@v1.1.1/command.go:854 +0x684
github.com/spf13/cobra.(*Command).ExecuteC(0xc000112848)
    /data/home/kylem/go/pkg/mod/github.com/spf13/cobra@v1.1.1/command.go:958 +0x389
github.com/spf13/cobra.(*Command).Execute(...)
    /data/home/kylem/go/pkg/mod/github.com/spf13/cobra@v1.1.1/command.go:895
github.com/metrumresearchgroup/bbi/cmd.Execute({0x8433b0?, 0x18?})
    /data/home/kylem/src/github/metrumresearchgroup/bbi/cmd/root.go:61 +0x153
main.main()
    /data/home/kylem/src/github/metrumresearchgroup/bbi/cmd/bbi/main.go:17 +0x25
reproducer [ assumes Metworx NONMEM installations. ] ```sh #!/bin/sh set -eu tdir=$(mktemp -d "${TMPDIR:-/tmp}"/nm73-param-XXXXXXX) cd "$tdir" url_base=https://raw.githubusercontent.com/metrumresearchgroup/bbi/v3.3.0/integration/testdata/acop curl -fSsL "$url_base/acop.mod" >1.mod curl -fSsLO "$url_base/acop.csv" bbi init --dir /opt/NONMEM run () { bbi nonmem run local --nm_version "$1" 1.mod mkdir "$1" mv 1 "$1/" cp 1.mod "$1/" } run nm75 run nm73gf grep -H 1000000007 nm75/1/1.ext grep -H 1000000007 nm73gf/1/1.ext || : printf '####################\n' bbi nonmem params --dir nm75 printf '####################\n' bbi nonmem params --dir nm73gf ```

This happens because params assumes a termination code has been extracted from the -1000000007:

https://github.com/metrumresearchgroup/bbi/blob/835953efed10fff2d7024ac1c3dd88949b268c05/cmd/params.go#L391-L392

The code responsible for extracting it handles the non-existent -1000000007 line okay:

https://github.com/metrumresearchgroup/bbi/blob/835953efed10fff2d7024ac1c3dd88949b268c05/parsers/nmparser/read_ext_fast.go#L50-L53


params should check that a termination code is present before indexing. This quick-and-dirty patch resolves the issue:

diff --git a/cmd/params.go b/cmd/params.go
index 456c4f0..5a288b6 100644
--- a/cmd/params.go
+++ b/cmd/params.go
@@ -388,8 +388,12 @@ func params(cmd *cobra.Command, args []string) {
                    s[idx] = values[len(values)-1][i]
                }

-               // first code is the termination status
-               terminationCode := results.TerminationCodes[0][0]
+               ncodes := len(results.TerminationCodes)
+               var terminationCode string
+               if ncodes > 0 {
+                   // The first item is the termination status.
+                   terminationCode = results.TerminationCodes[ncodes-1][0]
+               }
                fmt.Println(absolutePath + ",," + terminationCode + "," + strings.Join(s, ","))
            } else if res.Outcome == ERROR {
                errorMessage := res.Err.Error()

(Note that the patch also switches to using the code for the last estimation, to match where the -1000000000 result is taken from. And the results.TerminationCodes[ncodes-1][0] should be guarded to give an informative error if .TerminationCodes[ncodes-1] is unexpectedly empty.)