Skip to content
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

[audit] Make sure --audit output is reproducible across systems. #1788

Merged
merged 4 commits into from
Mar 25, 2019

Conversation

kumpera
Copy link
Collaborator

@kumpera kumpera commented Mar 13, 2019

Interactions output was using std::sort which triggered issues when values were the same.

Interactions output was using std::sort which triggered issues when values were the same.
@kumpera
Copy link
Collaborator Author

kumpera commented Mar 13, 2019

This fixes a bug I found while working on #1746

@JohnLangford
Copy link
Member

This looks good, but appveyor is barfing on 176?

@kumpera
Copy link
Collaborator Author

kumpera commented Mar 14, 2019

Investigating it next.

Copy link
Collaborator

@arielf arielf left a comment

Choose a reason for hiding this comment

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

LG.
Nice observation on the need for stable sort here.
Thanks!

@kumpera
Copy link
Collaborator Author

kumpera commented Mar 22, 2019

The appveyor failure was due to some technical debt in our test harness. I added an issue to track this: #1803

@jackgerrits jackgerrits merged commit ecccfe8 into VowpalWabbit:master Mar 25, 2019
jackgerrits pushed a commit to jackgerrits/vowpal_wabbit that referenced this pull request May 15, 2019
…palWabbit#1788)

* [audit] Make sure --audit output is reproducible across systems.

Interactions output was using std::sort which triggered issues when values were the same.

* [unitest] Disable acceptance test that can't be processed by RunTests.tt

* [tests] Disable 176 as well since it was being silently ignored until 177 was added.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants