Skip to content

Conversation

@michaelgsharp
Copy link
Contributor

The test TestBackAndForthConversionWithAlphaNoInterleave has stability issues on ARM/ARM64. Disabling the test only on those legs for CI stability. Tracked in #6043.

@michaelgsharp michaelgsharp requested review from a team, ericstj and safern January 12, 2022 17:39

using System.Runtime.InteropServices;

namespace Microsoft.ML.TestFramework.Attributes
Copy link
Member

@tarekgh tarekgh Jan 12, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In Runtime repo we usually use ConditionalFact and we pass there the condition instead of creating a new Fact attribute for different cases. Also, we have there the PlatformDetection which usually contain the properties for different configurations. Did we think to do something similar in machinelearning repo?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That makes a lot of sense. We don't really have anything like that in machinelearning. The idealistic goal is that there won't be any differences between the different architectures or OS's. When that does happen, it should be the rare exception to the rule. So I think thats why we don't have anything like that. I think using a ConditionalFact makes a lot of sense though. Can you point me to where you do something like that in Runtime and I'll make that change.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@codecov
Copy link

codecov bot commented Jan 12, 2022

Codecov Report

Merging #6044 (07526df) into main (ae97ece) will decrease coverage by 0.00%.
The diff coverage is 0.00%.

@@            Coverage Diff             @@
##             main    #6044      +/-   ##
==========================================
- Coverage   68.54%   68.53%   -0.01%     
==========================================
  Files        1146     1146              
  Lines      245450   245451       +1     
  Branches    25637    25638       +1     
==========================================
- Hits       168232   168216      -16     
- Misses      70507    70523      +16     
- Partials     6711     6712       +1     
Flag Coverage Δ
Debug 68.53% <0.00%> (-0.01%) ⬇️
production 63.32% <ø> (-0.01%) ⬇️
test 88.65% <0.00%> (-0.02%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
test/Microsoft.ML.Tests/ImagesTests.cs 97.99% <0.00%> (-0.14%) ⬇️
...crosoft.ML.AutoML/Experiment/Runners/RunnerUtil.cs 60.00% <0.00%> (-16.00%) ⬇️
...oML/Experiment/MetricsAgents/BinaryMetricsAgent.cs 74.35% <0.00%> (-7.70%) ⬇️
...AutoML/Experiment/Runners/CrossValSummaryRunner.cs 68.53% <0.00%> (-3.50%) ⬇️
....ML.AutoML/PipelineSuggesters/PipelineSuggester.cs 78.74% <0.00%> (-3.15%) ⬇️
test/Microsoft.ML.AutoML.Tests/AutoFitTests.cs 84.55% <0.00%> (-2.53%) ⬇️
src/Microsoft.ML.Core/Data/IHostEnvironment.cs 95.12% <0.00%> (-2.44%) ⬇️
src/Microsoft.ML.Core/Utilities/Contracts.cs 45.27% <0.00%> (-0.21%) ⬇️
...ML.Transforms/Text/StopWordsRemovingTransformer.cs 86.23% <0.00%> (-0.15%) ⬇️
...StandardTrainers/Standard/LinearModelParameters.cs 66.07% <0.00%> (+0.25%) ⬆️
... and 2 more

@michaelgsharp michaelgsharp merged commit 8eac93a into dotnet:main Jan 14, 2022
@michaelgsharp michaelgsharp deleted the test-disable branch January 14, 2022 22:52
ericstj pushed a commit to ericstj/machinelearning that referenced this pull request Mar 7, 2022
@ghost ghost locked as resolved and limited conversation to collaborators Mar 17, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants