CountFeatureSelectingEstimator no selection support#5000
CountFeatureSelectingEstimator no selection support#5000kere-nel merged 6 commits intodotnet:masterfrom
Conversation
| string opType; | ||
| if (_srcTypes[iinfo] is VectorDataViewType) | ||
| var slots = _slotDropper[iinfo].GetPreservedSlots(); | ||
| if (_srcTypes[iinfo] is VectorDataViewType && slots.Count() > 0) |
There was a problem hiding this comment.
_srcTypes[iinfo] is VectorDataViewType && slots.Count() > 0 [](start = 20, length = 59)
what if _srcTypes[iinfo] is VectorDataViewType && slots.Count() == 0? should we return true or false?
in current implementation seems it will goes into the "else" condition in line 917 and which should handling double numeric data type #Resolved
There was a problem hiding this comment.
If slots.Count == 0, then the column get's "suppressed". It's not handled well by the GatherElements operator, so we create an empty vector in the else clause. Does that make sense?
There was a problem hiding this comment.
yes, but we better to make the logic clear like below, don't let different cases fall into same condition which will make code harder to read and will cause potential issue in future:
if (_srcTypes[iinfo] is VectorDataViewType)
{
if (count > 0) { ... }
else { ... }
}
else { ... }
In reply to: 404404988 [](ancestors = 404404988)
Issue: The GatherElements ONNX operator used by SlotsDroppingTransformer (transformer for CountFeatureSelectingEstimator) has issues when there's nothing selected, so mlnet needs to handle this case separately.
This PR
Other issues: