`PipeOpNMF`'s `.select_cols` does not handle `NA`s gracefully
```R
library(mlr3)
library(mlr3pipelines)
op = po("nmf")
op$train(list(tsk("penguins")))
# Error in `.__Task__data()`:
# ! Assertion on 'cols' failed: Must be a subset of {'..row_id','bill_depth','bill_length','body_mass','flipper_length','island','sex','species','year'}, but has additional elements {'NA'}.
# This happened in PipeOp nmf's $train()
# Hide Traceback
# ▆
# 1. ├─op$train(list(tsk("penguins")))
# 2. │ └─mlr3pipelines:::.__PipeOp__train(...)
# 3. │ ├─base::withCallingHandlers(...) at mlr3pipelines/R/PipeOp.R:315:7
# 4. │ └─private$.train(input) at mlr3pipelines/R/PipeOp.R:316:9
# 5. │ └─mlr3pipelines:::.__PipeOpTaskPreproc__.train(...)
# 6. │ └─private$.train_task(intask) at mlr3pipelines/R/PipeOpTaskPreproc.R:211:7
# 7. │ └─mlr3pipelines:::.__PipeOpTaskPreproc__.train_task(...)
# 8. │ └─task$data(cols = cols) at mlr3pipelines/R/PipeOpTaskPreproc.R:267:7
# 9. │ └─mlr3:::.__Task__data(...)
# 10. │ └─checkmate::assert_subset(cols, self$col_info$id)
# 11. │ └─checkmate::makeAssertion(x, res, .var.name, add)
# 12. │ └─checkmate:::mstop(...)
# 13. │ └─base::stop(simpleError(sprintf(msg, ...), call.))
# 14. └─mlr3pipelines (local) `<fn>`(`<smplErrr>`)
# mlr_pipeops$add("nmf", PipeOpNMF)
```
The source of the error lies here:
https://github.com/mlr-org/mlr3pipelines/blob/0ff0e8f8066c89a90a0fb29ab5525f5be10b9d43/R/PipeOpNMF.R#L206-L211
Here, if a feature column contains `NA`s, `all(x >= 0)` will return an NA. This is then part of the return vector, which should be a vector of column names.
The solution to this depends on whether `NMF::nmf()` can handle `NA`s. If no feature fits our criterion (non-negative and potentially fully observed), we probably don't want an error and instead just continue with the PipeOp not doing anything? It would be good to add an early exit in the train and predict methods in that case.
2 条评论