Remove parallel for from data table to fix computed column threading issue in #1831 #1835
Conversation
timkpaine
added
bug
| @@ -208,7 +208,7 @@ class PERSPECTIVE_EXPORT t_gnode { | |||
| std::shared_ptr<t_expression_vocab> get_expression_vocab() const; | |||
| std::shared_ptr<t_regex_mapping> get_expression_regex_mapping() const; | |||
|
|
|||
| #ifdef PSP_PARALLEL_FOR | |||
| #ifdef PSP_ENABLE_PYTHON | |||
There was a problem hiding this comment.
this is unrelated but should be enabled conditional on PYTHON, not PARALLEL_FOR
|
@texodus might need your help benchmarking this, if it causes a substantial degradation in performance I can try to break this loop apart to reenable some part in parallel |
There was a problem hiding this comment.
It's not clear to my why this change would impact the test provided, so on a hunch I forked just this test to a new branch and it passes just the same on all Linux builds. What am I missing?
|
This test fails on my Linux machine, validated on 2 other Linux machines |
|
See #1831 for more details |
|
My guess would be either azure or the manylinux images themselves don't trigger the test case to fail. |
|
Some additional testing suggests this might've been fixed already via the arrow upgrade #1816 |
|
So thanks to the accompanying test (which I do want to merge), I can confirm that this issue was resolved by #1793, and is unrelated to Arrow/threading. Fix version is |
No description provided.