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

[Fix] Fix flaky test of #9952 #9958

Conversation

wrongtest-intellif
Copy link
Contributor

fix #9952

@MasterJH5574
Copy link
Contributor

Thanks @wrongtest for the fix! If you can make sure the indeterminism doesn't happen again, please also kindly revert this hot fix PR (#9956) by removing the "skip" mark and the pytest import.

cc @junrushao1994 @Hzfengsy

Copy link
Contributor

@MasterJH5574 MasterJH5574 left a comment

Choose a reason for hiding this comment

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

LGTM. Please take a look at my comment, thanks!

Comment on lines 283 to 288
for (const Var& var : cand_vars) {
if (!var_set.count(var)) {
vars.push_back(var);
var_set.insert(var);
}
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Personally I recommend using std::find(vars.begin(), vars.end(), var) != vars.end() instead of using a var_set for looking up, because usually we don't have few variables and the map doesn't bring performance improvement :-)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

std::find(vars.begin(), vars.end(), var) != vars.end()

Thanks for the advice~ I would change to O(n) search and commit to retrigger CI again.

Copy link
Member

@junrushao junrushao left a comment

Choose a reason for hiding this comment

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

Thanks for fixing this!

@junrushao junrushao merged commit 611b430 into apache:main Jan 19, 2022
yuanfz98 pushed a commit to yuanfz98/tvm that referenced this pull request Jan 24, 2022
* fix to stablize the var orders when solve bounds in region analysis

* change to std::find_if since num of vars is generally small
crazydemo pushed a commit to crazydemo/tvm that referenced this pull request Jan 27, 2022
* fix to stablize the var orders when solve bounds in region analysis

* change to std::find_if since num of vars is generally small
ylc pushed a commit to ylc/tvm that referenced this pull request Feb 16, 2022
* fix to stablize the var orders when solve bounds in region analysis

* change to std::find_if since num of vars is generally small
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants