Skip to content

Update test baseline for TimeSeries tests #4756

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 1 commit into from

Conversation

sharwell
Copy link
Member

Extracted from #4569

@sharwell sharwell requested a review from a team as a code owner January 31, 2020 19:47
@sharwell sharwell added the test related to tests label Jan 31, 2020
@@ -163,8 +169,13 @@ public void ChangePointDetectionWithSeasonality()
// Get predictions
var enumerator = env.Data.CreateEnumerable<Prediction>(output, true).GetEnumerator();
Prediction row = null;
#if NETCOREAPP3_0
Copy link
Contributor

Choose a reason for hiding this comment

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

Could you please help me understand the root cause of why netcore 3.0 generates different result?

Copy link
Member

Choose a reason for hiding this comment

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

I am curious to know as well :)

Copy link
Member Author

Choose a reason for hiding this comment

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

Copy link
Contributor

Choose a reason for hiding this comment

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

@tannergooding / @sharwell Any updates on this? It would be really good to know why netcore3.0 would generate different results. It may help with other investigations?

Copy link
Contributor

Choose a reason for hiding this comment

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

I appears that previously the tests werent running when .net version was less than 3.0. And @sharwell's fix improves that by getting the tests to run on both again but with different baselines.

@frank-dong-ms: Can we accept this PR and re-investigate the issue of different baselines later?

@sharwell Can you please amend this PR with an // TODO: [TEST_STABILITY] comment? We are using that tag to track issues in code that still need to be addressed beyond outright test failures?

@harishsk
Copy link
Contributor

harishsk commented Apr 6, 2020

@sharwell Can you please fix the merge conflicts and address the small change requested in my last comment so we can merge this PR?

@harishsk
Copy link
Contributor

@sharwell Can you please update the PR according to the note above?

@harishsk
Copy link
Contributor

@frank-dong-ms : Can you please review this PR and close it if you have already incorporated all these changes into your prior PRs?

@frank-dong-ms-zz
Copy link
Contributor

This is done through our work of all legacy tests. We created separate baseline/expected result for some tests that generates different result for netcore app 3.1. The reason of these difference comes from CPUMath library, for netcoreapp 3.1 we are using AVX instruction set and for netcoreapp2.1 and netfx we are using SSE instruction set and these 2 instruction set generates slightly different results.

@ghost ghost locked as resolved and limited conversation to collaborators Mar 19, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
test related to tests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants