Conversation
Codecov Report
@@ Coverage Diff @@
## master #3693 +/- ##
=========================================
Coverage ? 72.57%
=========================================
Files ? 811
Lines ? 146001
Branches ? 16279
=========================================
Hits ? 105958
Misses ? 35620
Partials ? 4423
|
|
Can we be sure to add some tests for this new feature when it is ready? |
|
@eerhardt this is a work in progress PR. Let’s give the user the chance to remove the draft tag before we start reviewing. I’m working with Meng and she will add tests and even performance benchmarks. |
|
Could you also point to some benchmarks/tests where you show that it performs well? Either in this PR or in another we will also need to make sure we have samples for this trainer. |
Sure. I'm now working on the benchmark now. And the samples are provided already, I will add more descriptions and comments. |
1.Add xml to DetectAnomalyBySrCnn; 2.Add default value to DetectAnomalyBySrCnn; 3.Change class name of SrCnnAnomalyDetectionBaseWrapper; 4.fix AlertThreshold write bug; 5.Add check argument part and remove redundent TODOs 6.Fix slot name filling bug.
docs/samples/Microsoft.ML.Samples/Dynamic/Transforms/TimeSeries/DetectAnomalyBySrCnn.cs
Outdated
Show resolved
Hide resolved
docs/samples/Microsoft.ML.Samples/Dynamic/Transforms/TimeSeries/DetectAnomalyBySrCnn.cs
Outdated
Show resolved
Hide resolved
docs/samples/Microsoft.ML.Samples/Dynamic/Transforms/TimeSeries/DetectAnomalyBySrCnn.cs
Show resolved
Hide resolved
...es/Microsoft.ML.Samples/Dynamic/Transforms/TimeSeries/DetectAnomalyBySrCnnBatchPrediction.cs
Outdated
Show resolved
Hide resolved
...es/Microsoft.ML.Samples/Dynamic/Transforms/TimeSeries/DetectAnomalyBySrCnnBatchPrediction.cs
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
The code framework looks good. Just two points for improvements needed in next PR.
- Please add prediction equation in another PR. You can use Latex syntax.
- Please try to avoid frequent
float[]allocation in C# whenever possible. I saw some inState's function.
Thanks!
Add SRCNN algorithm to TimSeries Anomaly Detection
fixes #3799