Inconsistent NA return from failed subproblems

[josherrickson]

When there are multiple subproblems, sometimes failed matches return NA and sometimes return 1.NA or some other prefix, which R doesn't recognize as NA. This is not consistent and it breaks matchfailed via subproblemSuccess.

First, things work fine with no subproblems.

> (f1 <- fullmatch(pr ~ cost, data = nuclearplants, min = 5, max = 5))
   H    I    A    J    B    K    L    M    C    N    O    P 
<NA> <NA> <NA> <NA> <NA> <NA> <NA> <NA> <NA> <NA> <NA> <NA> 
   Q    R    S    T    U    D    V    E    W    F    X    G 
<NA> <NA> <NA> <NA> <NA> <NA> <NA> <NA> <NA> <NA> <NA> <NA> 
   Y    Z    d    e    f    a    b    c 
<NA> <NA> <NA> <NA> <NA> <NA> <NA> <NA> 
> matchfailed(f1)
 [1] TRUE TRUE TRUE TRUE TRUE TRUE TRUE TRUE TRUE TRUE TRUE
[12] TRUE TRUE TRUE TRUE TRUE TRUE TRUE TRUE TRUE TRUE TRUE
[23] TRUE TRUE TRUE TRUE TRUE TRUE TRUE TRUE TRUE TRUE

However, with subproblems, things get weird.

> (f2 <- fullmatch(pr ~ cost, data = nuclearplants, min = 5, max = 5, 
+                 within = exactMatch(pr ~ pt, data = nuclearplants)))
   H    I    A    J    B    K    L    M    C    N    O    P 
<NA> <NA> <NA> <NA> <NA> <NA> <NA> <NA> <NA> <NA> <NA> <NA> 
   Q    R    S    T    U    D    V    E    W    F    X    G 
<NA> <NA> <NA> <NA> <NA> <NA> <NA> <NA> <NA> <NA> <NA> <NA> 
   Y    Z    d    e    f    a    b    c 
<NA> <NA> 1.NA 1.NA 1.NA 1.NA 1.NA 1.NA 
> matchfailed(f2)
 [1]  TRUE  TRUE  TRUE  TRUE  TRUE  TRUE  TRUE  TRUE  TRUE
[10]  TRUE  TRUE  TRUE  TRUE  TRUE  TRUE  TRUE  TRUE  TRUE
[19]  TRUE  TRUE  TRUE  TRUE  TRUE  TRUE  TRUE  TRUE FALSE
[28] FALSE FALSE FALSE FALSE FALSE
> 
> (f3 <- fullmatch(pr ~ cost, data = nuclearplants, min = 60, max = 60, 
+                 within = exactMatch(pr ~ pt, data = nuclearplants)))
   H    I    A    J    B    K    L    M    C    N    O    P 
0.NA 0.NA 0.NA 0.NA 0.NA 0.NA 0.NA 0.NA 0.NA 0.NA 0.NA 0.NA 
   Q    R    S    T    U    D    V    E    W    F    X    G 
0.NA 0.NA 0.NA 0.NA 0.NA 0.NA 0.NA 0.NA 0.NA 0.NA 0.NA 0.NA 
   Y    Z    d    e    f    a    b    c 
0.NA 0.NA 1.NA 1.NA 1.NA 1.NA 1.NA 1.NA 
> matchfailed(f3)
 [1] FALSE FALSE FALSE FALSE FALSE FALSE FALSE FALSE FALSE
[10] FALSE FALSE FALSE FALSE FALSE FALSE FALSE FALSE FALSE
[19] FALSE FALSE FALSE FALSE FALSE FALSE FALSE FALSE FALSE
[28] FALSE FALSE FALSE FALSE FALSE

[josherrickson]

One way to address this downstream would be to modify subproblemSuccess and replace

 all(is.na(x))

with

all(is.na(x) | grepl("NA$", x))

Given the inconsistency of NA vs #.NA, I'm not sure which is intended. If #.NA is proper, this fix is necessary. If #.NA is a bug, then we should address that upstream.

[benthestatistician]

If #.NA is a bug, then we should address that upstream.

I suspect that's a bug, but I'm hoping @markmfredrickson will weigh in on this too. (That's the way much earlier versions of optmatch flagged failed matches, if memory served.)

…

[benthestatistician]

Reviewing this code again now, I'd support the modification to subproblemSuccess that was proposed in Josh's last comment here, namely replacing all(is.na(x)) with all(is.na(x) | grepl("NA$", x)). In other words, move the material aeaa6a2 inserted near the end of fullmatch.matrix into subproblemSuccess.

[josherrickson]

I made these modifications by adjusting subproblemSuccess's logical check and simplifying the bit in fullmatch appropriately.

I also expanded the tests for subproblemSuccess and added tests for matchfailed to hopefully get ahead of any further similar issues.

Leaving the issue open for @markmfredrickson to chime in as the NA/#.NA discrepancy is still concerning.

[benthestatistician]

Thanks for this follow-up, Josh!

Rather than inferring subproblem failure or success from whether all the matches are NA, or #.NA, it would be better to more directly pass the solver's answer to this question up the call stack and have subproblemSuccess just look at it. The issue54-hinting branch does some things that should enable this: in particular, passing up a table of subproblem information, one column of which notes whether the subproblem was found to be feasible or not. I've opened a new issue (#178) for that purpose, and am going to close out this one (but Mark don't let that stop you from sharing anything you might like to add).