Rebase on numba 0.46 #224

AlexanderKalistratov · 2019-10-14T12:16:01Z

No description provided.

Hardcode84 · 2019-10-18T09:42:49Z

hpat/decorators.py

@@ -47,6 +47,8 @@ def jit(signature_or_function=None, **options):
        return numba.jit(signature_or_function, **options)

    _locals = options.pop('locals', {})
+    # print("\nlocals: " + str(_locals))


Hardcode84 · 2019-10-18T09:43:00Z

hpat/decorators.py

@@ -93,4 +95,5 @@ def jit(signature_or_function=None, **options):
    # this is for previous version of pipeline manipulation (numba hpat_req <0.38)
    # from .compiler import add_hpat_stages
    # return numba.jit(signature_or_function, user_pipeline_funcs=[add_hpat_stages], **options)
+    # return numba.jit(signature_or_function, pipeline_class=hpat.compiler.HPATPipeline, **options)


Hardcode84 · 2019-10-18T09:44:26Z

hpat/distributed_analysis.py

@@ -68,6 +68,9 @@ def _init_run(self):
        self.second_pass = False
        self.in_parallel_parfor = -1

+    def run_pass(self, state):


remove and call run directly

I think run_pass(self, state) is interface function. It is better to inline run() here.

Hardcode84 · 2019-10-18T09:45:30Z

hpat/tests/test_groupby.py

@@ -394,6 +396,25 @@ def test_impl(df):
        hpat_func = hpat.jit(test_impl)
        pd.testing.assert_frame_equal(hpat_func(df), test_impl(df))

+    def test_df_nonliteral(self):


skip this test for now?

Hardcode84 · 2019-10-18T09:45:42Z

hpat/tests/test_series.py

@@ -67,9 +68,19 @@ class TestSeries(unittest.TestCase):

    def test_create1(self):
        def test_impl():
+            # a = 2


Hardcode84 · 2019-10-18T09:51:10Z

I assume we need to update numba version in our public ci as well

Hardcode84 · 2019-10-18T09:52:29Z

Also, disable tests broken by parfor issue as were disscussed

PokhodenkoSA · 2019-10-18T10:00:00Z

hpat/tests/test_series.py

@@ -67,9 +68,19 @@ class TestSeries(unittest.TestCase):

    def test_create1(self):
        def test_impl():
+            # a = 2


Suggested change

# a = 2

@Hardcode84
There is code suggestion tool in GitHub (ctrl+g)

PokhodenkoSA · 2019-10-18T10:01:30Z

hpat/tests/test_series.py

@@ -4,6 +4,7 @@
 import numpy as np
 import pyarrow.parquet as pq
 import hpat
+import numba


Suggested change

import numba

import numba

What is the difference?

PokhodenkoSA · 2019-10-18T10:02:39Z

hpat/tests/test_groupby.py

@@ -8,6 +8,8 @@
                                   count_parfor_OneDs, count_array_OneDs, dist_IR_contains,
                                   get_start_end)

+from numba.special import literally


Is it used somewhere?

no. Removed

densmirn · 2019-10-18T10:09:35Z

hpat/compiler.py

+    for idx, (x, _) in enumerate(pm.passes):
+        if x == location:
+            return idx
+    else:


else: is excess and could be removed because we don't have break in the loop. Let's leave only raise.

densmirn · 2019-10-18T10:16:37Z

hpat/compiler.py

+    def __init__(self):
+        pass


Looks like it should work the same way without the stub.

__init__ is marked as an abstract method in the base class

densmirn · 2019-10-18T10:18:15Z

hpat/compiler.py

+    def __init__(self):
+        pass


Looks like it should work the same way without the constructor, constructor of the parent class will be invoked anyway.

__init__ is marked as an abstract method in the base class

densmirn · 2019-10-18T10:27:30Z

hpat/hiframes/aggregate.py

+    state = namedtuple('State',
+                            ['typingctx', 'targetctx', 'args', 'func_ir', 'typemap', 'return_type',
+                                'calltypes', 'metadata'])


Just in case there is another way to define named tuple:

from typing import NamedTuple class State(NamedTuple): typingctx: <type> targetctx: <type> args: <type> func_ir: <type> typemap: <type> return_type: <type> calltypes: <type> metadata: <type>

If you don't have strong opinion it should be modified, I'd prefer to left as is

Hardcode84 · 2019-10-18T13:10:58Z

hpat/tests/test_basic.py

@@ -297,6 +297,7 @@ def test_array_reduce(self):
            self.assertEqual(count_array_OneDs(), 0)
            self.assertEqual(count_parfor_OneDs(), 1)

+    @unittest.skipIf(numba.__version__.startswith('0.46.'), "Broken in numba 0.46.x")


add url to issue numba/numba#4690

And IMO better to check exact version, e.g. if it will be still broken on 0.46.1 we will see that

Also, maybe create some helper function to check numba version to avoid copypaste

shssf · 2019-10-18T13:16:28Z

hpat/hiframes/dataframe_pass.py

-    def run(self):
-        blocks = self.func_ir.blocks
+    def __init__(self, state):
+        self.state = state


self.func_ir, self.typemap, self.typingctx = self
?
no need to keep new variable in this case

shssf · 2019-10-18T13:17:10Z

hpat/distributed.py

+
+    .. only:: developer
+
+        The content for **Developer's Guide** should go here


It would be great to have any documentation

shssf

It looks like this PR should be separated to, at least, two PRs. First PR is preparation work. For example, to rename variables self.typemap to self.state.typemap. It will not depend on Numba version

AlexanderKalistratov · 2019-10-22T12:58:19Z

Split PR into 5 commits:

Update globals for generated function
Replace numba.compiler with numba.typed_pass
Rename self. to self.state.
Modify aggregate.py to support state
All other changes

Most of commits would not work on numba 0.45 and doesn't work separately from each other, so all commits must be squashed before merge

shssf · 2019-10-22T19:17:51Z

hpat/utils.py

@@ -531,3 +532,7 @@ def dump_node_list(node_list):

 def debug_prints():
    return numba.config.DEBUG_ARRAY_OPT == 1
+
+def update_globals(func, glbls):


it looks like this function used nowhere. please delete

It is used in distributed, dataframe, high frame typad & untyped passes. It updates globals in generated functions. It was introduced in order to restore functionality of inline_closure_call which doesn't update globals anymore.

You can see details in related commit: 4b828c9

shssf · 2019-10-22T19:18:27Z

hpat/utils.py

@@ -18,6 +18,7 @@
 from hpat.str_ext import string_type, list_string_array_type
 from hpat.str_arr_ext import string_array_type, num_total_chars, pre_alloc_string_array
 from enum import Enum
+import types as pytypes


is it used in this module?

It is used in update_globals function

shssf · 2019-10-22T19:21:54Z

hpat/tests/test_utils.py

@@ -53,3 +53,6 @@ def get_start_end(n):
    start = hpat.distributed_api.get_start(n, n_pes, rank)
    end = hpat.distributed_api.get_end(n, n_pes, rank)
    return start, end
+
+def check_numba_version(version):
+    return numba.__version__ == version


numba.__version__ is shorter than check_numba_version
may be it is better to use this variable from Numba directly inplace?

I don't know. For me both versions are fine, though version with function is a little bit better, since it doesn't looks so ugly and encapsulates details

AlexanderKalistratov · 2019-10-23T13:35:26Z

Rebased & squashed

AlexanderKalistratov changed the title ~~rebase on numba 0.46~~ WIP: rebase on numba 0.46 Oct 14, 2019

densmirn self-requested a review October 14, 2019 12:24

shssf added the [WIP] Work in progress label Oct 14, 2019

AlexanderKalistratov requested review from densmirn and removed request for densmirn October 14, 2019 15:07

Hardcode84 mentioned this pull request Oct 15, 2019

numba 0.46, useless(?) code in parfor init block numba/numba#4690

Closed

2 tasks

Hardcode84 reviewed Oct 18, 2019

View reviewed changes

PokhodenkoSA reviewed Oct 18, 2019

View reviewed changes

densmirn reviewed Oct 18, 2019

View reviewed changes

Hardcode84 reviewed Oct 18, 2019

View reviewed changes

shssf reviewed Oct 18, 2019

View reviewed changes

shssf suggested changes Oct 18, 2019

View reviewed changes

AlexanderKalistratov force-pushed the akalist/rebase_on_numba branch from 4bfb01c to 945efb5 Compare October 22, 2019 12:51

AlexanderKalistratov changed the title ~~WIP: rebase on numba 0.46~~ Rebase on numba 0.46 Oct 22, 2019

AlexanderKalistratov removed the [WIP] Work in progress label Oct 22, 2019

AlexanderKalistratov force-pushed the akalist/rebase_on_numba branch from 945efb5 to 9cda957 Compare October 22, 2019 13:33

shssf reviewed Oct 22, 2019

View reviewed changes

shssf approved these changes Oct 22, 2019

View reviewed changes

rebase HPAT on Numba 0.46

2c0548e

AlexanderKalistratov force-pushed the akalist/rebase_on_numba branch from ea2a9b0 to 2c0548e Compare October 23, 2019 13:34

densmirn approved these changes Oct 24, 2019

View reviewed changes

shssf merged commit 8e3a220 into IntelPython:master Oct 25, 2019


		.. only:: developer

		The content for Developer's Guide should go here

Rebase on numba 0.46 #224

Rebase on numba 0.46 #224

Conversation

AlexanderKalistratov commented Oct 14, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Hardcode84 commented Oct 18, 2019

Hardcode84 commented Oct 18, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

PokhodenkoSA Oct 18, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Hardcode84 Oct 18, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

shssf left a comment

Choose a reason for hiding this comment

AlexanderKalistratov commented Oct 22, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

AlexanderKalistratov commented Oct 23, 2019

PokhodenkoSA Oct 18, 2019 •

edited

Loading

Hardcode84 Oct 18, 2019 •

edited

Loading