Draft PR for SrCnn batch detection API interface #1
Conversation
src/Microsoft.ML.Data/DataView/DetectAnomalyBySrCnnBatchTransform.cs
Outdated
Show resolved
Hide resolved
src/Microsoft.ML.Data/DataView/DetectAnomalyBySrCnnBatchTransform.cs
Outdated
Show resolved
Hide resolved
src/Microsoft.ML.Data/DataView/DetectAnomalyBySrCnnBatchTransform.cs
Outdated
Show resolved
Hide resolved
| var active = SchemaBindings.GetActive(predicate); | ||
| Contracts.Assert(active.Length == SchemaBindings.ColumnCount); | ||
|
|
||
| // REVIEW: We can get a different input predicate for the input cursor and for the lookahead cursor. The lookahead |
There was a problem hiding this comment.
REVIEW [](start = 15, length = 6)
@harishsk, do you think it is ok to leave this as a review comment, or should this be addressed?
To follow the suggestion in the comment we would need to introduce an abstract method that gets the active columns for the lookahead cursor - in the simple implementation it would simply return _source.Schema[SchemaBindings._inputColumnIndex].
There was a problem hiding this comment.
src/Microsoft.ML.Data/DataView/DetectAnomalyBySrCnnBatchTransform.cs
Outdated
Show resolved
Hide resolved
src/Microsoft.ML.Data/DataView/DetectAnomalyBySrCnnBatchTransform.cs
Outdated
Show resolved
Hide resolved
src/Microsoft.ML.Data/DataView/DetectAnomalyBySrCnnBatchTransform.cs
Outdated
Show resolved
Hide resolved
src/Microsoft.ML.Data/DataView/DetectAnomalyBySrCnnBatchTransform.cs
Outdated
Show resolved
Hide resolved
|
|
||
| protected override Batch InitializeBatch(DataViewRowCursor input) => new Batch(_batchSize); | ||
|
|
||
| protected override Func<bool> GetIsNewBatchDelegate(DataViewRowCursor input) |
There was a problem hiding this comment.
GetIsNewBatchDelegate [](start = 38, length = 21)
After giving it some thought, I think GetIsNewBatchDelegate and GetLastInBatchDelegate can be implemented privately in the base class.
(This computation is not specific to SrCnn, it would be done the same way for any mapper that uses a constant batch size. If in the future there will be a need to introduce a different way of computing these, we can refactor the base class).
There was a problem hiding this comment.
But these are used in BatchDataViewMapperBase.Cursor?
* Draft PR for SrCnn batch detection API interface (#1) * POC Batch transform * SrCnn batch interface * Removed comment * Handled some APIreview comments. * Handled other review comments. * Resolved review comments. Added sample. Co-authored-by: Yael Dekel <[email protected]> * Implement SrCnn entire API by function * Fix bugs and add test * Resolve comments * Change names and add documentation * Handling review comments * Resolve the array allocating issue * Move modeler initializing to CreateBatch and other minor fix. * Fix 3 remaining comments * Fixed code analysis issue. * Fixed minor comments Co-authored-by: klausmh <[email protected]> Co-authored-by: Yael Dekel <[email protected]>
6 participants
No description provided.