Replace assertion errors on IR assumption violation by sklam · Pull Request #7223 · numba/numba

sklam · 2021-07-20T20:13:00Z

with pedantic warnings.

stuartarchibald · 2021-07-21T12:18:16Z

+                if newtarget.name not in scope.localvars:
+                    wmsg = (
+                        f"variable name {newtarget.name!r} not in scope "
+                        f"in {assign} on line {assign.loc}",
+                    )
+                    warnings.warn(errors.NumbaIRAssumptionWarning(wmsg))


Suggested change

if newtarget.name not in scope.localvars:

wmsg = (

f"variable name {newtarget.name!r} not in scope "

f"in {assign} on line {assign.loc}",

)

warnings.warn(errors.NumbaIRAssumptionWarning(wmsg))

if newtarget.name not in scope.localvars:

wmsg = f"variable name {newtarget.name!r} not in scope"

warnings.warn(errors.NumbaIRAssumptionWarning(wmsg, loc=assign.loc))

Does it help to use Numba's error message loc handling here?

oh, i forgot that it has that

changed in ec01acd

stuartarchibald · 2021-07-21T19:24:26Z

Note: verified behaviour in ec01acd against #7217 (comment)

stuartarchibald

Thanks for the patch. There's a few minor things to resolve but looks good else.

stuartarchibald · 2021-07-21T19:28:13Z

+        # import logging
+        # logging.basicConfig(level=logging.DEBUG)


Dead comments

Co-authored-by: stuartarchibald <stuartarchibald@users.noreply.github.com>

stuartarchibald

Couple of suggestions to hopefully fix the build.

stuartarchibald · 2021-07-21T20:48:43Z

+        # import logging
+        # logging.basicConfig(level=logging.DEBUG)


Suggested change

# import logging

# logging.basicConfig(level=logging.DEBUG)

stuartarchibald · 2021-07-21T20:49:10Z

+            # Verify the error message
+            self.assertRegex(
+                str(raises.exception),
+                r"variable name '[a-z]' not in scope",


Suggested change

r"variable name '[a-z]' not in scope",

r"variable '[a-z]' is not in scope",

sklam · 2021-07-21T21:14:29Z

Btw, i'm trying out if the warning message should suggest users to report.

Currently, the warning looks like:

/path/to/numba/core/ssa.py:272: NumbaIRAssumptionWarning: variable 'a' is not in scope.
  warnings.warn(errors.NumbaIRAssumptionWarning(wmsg,
/path/to/numba/core/ssa.py:272: NumbaIRAssumptionWarning: variable 'b' is not in scope.
  warnings.warn(errors.NumbaIRAssumptionWarning(wmsg,

With the suggestion:

/path/to/numba/core/ssa.py:272: NumbaIRAssumptionWarning: variable 'b' is not in scope.
This error came from an internal pedantic check. Please report the error message
and traceback, along with a minimal reproducer at:
https://github.com/numba/numba/issues/new

  warnings.warn(errors.NumbaIRAssumptionWarning(wmsg,
/path/to/numba/core/ssa.py:272: NumbaIRAssumptionWarning: variable 'a' is not in scope.
This error came from an internal pedantic check. Please report the error message
and traceback, along with a minimal reproducer at:
https://github.com/numba/numba/issues/new

  warnings.warn(errors.NumbaIRAssumptionWarning(wmsg,

The long message can be very annoying.

stuartarchibald · 2021-07-21T21:27:04Z

RE #7223 (comment) Whilst the long messages are not great, I'd hope that the frequency at which this is hit is really small else it indicates a lot more problems. I guess this comes down to our confidence around having got Numba's internals sufficiently fixed up for this to be a rarity and definitely something that should be reported vs. an annoyance?

stuartarchibald · 2021-07-21T21:29:07Z

+
+            def run_pass(self, state):
+                # This is unreachable. SSA pass should have raised before this
+                # pass when run with `error.NumbaPedanicWarning`s raised as


Suggested change

# pass when run with `error.NumbaPedanicWarning`s raised as

# pass when run with `error.NumbaPedanticWarning`s raised as

stuartarchibald · 2021-07-22T15:32:06Z


+
+pedantic_warning_info = """
+This error came from an internal pedantic check. Please report the error message


Suggested change

This error came from an internal pedantic check. Please report the error message

This warning came from an internal pedantic check. Please report the warning message

(this will need a line wrap)

sklam · 2021-07-27T20:03:37Z

@esc, can you review this? It's just need a last look.

esc

A few minor things to edit, ready to merge after that.

esc · 2021-07-29T13:35:24Z

+                return [pm]
+
+        @njit(pipeline_class=MyCompiler)
+        def dummy(x):


Perhaps add a comment explaining why this dummy is needed: multiple assignments to the same variable name are needed to trigger SSA.

done in 3d6fb07

Co-authored-by: esc <esc@users.noreply.github.com>

esc · 2021-07-30T11:38:06Z

+
 feedback_details = """
 Please report the error message and traceback, along with a minimal reproducer
 at: https://github.com/numba/numba/issues/new


Do you want to update this URL too, while there?

Let's do this in a future PR.

i've made a task issue for it: #7261

esc

Changes are fine, I left one comment but that is perhaps something for a future PR.

Replace assertion errors on IR assumption violation

Replace assertion errors on IR assumption violation

359b5b0

with pedantic warnings.

sklam added this to the Numba 0.54 RC3 milestone Jul 20, 2021

sklam added 2 commits July 20, 2021 17:05

pep8

e307287

pep8

34dea22

stuartarchibald reviewed Jul 21, 2021

View reviewed changes

Use loc in warning class instead

ec01acd

sklam marked this pull request as ready for review July 21, 2021 15:01

sklam requested a review from esc as a code owner July 21, 2021 15:01

sklam added the 3 - Ready for Review label Jul 21, 2021

stuartarchibald reviewed Jul 21, 2021

View reviewed changes

stuartarchibald added 4 - Waiting on author Waiting for author to respond to review Effort - short Short size effort needed and removed 3 - Ready for Review labels Jul 21, 2021

sklam and others added 2 commits July 21, 2021 15:40

Apply suggestions from code review

30a4cd2

Co-authored-by: stuartarchibald <stuartarchibald@users.noreply.github.com>

Remove dead comments

d67e05d

stuartarchibald reviewed Jul 21, 2021

View reviewed changes

Update test to new error message

5463998

stuartarchibald reviewed Jul 21, 2021

View reviewed changes

sklam added 2 commits July 22, 2021 10:11

Fix typo

9ffdd3d

Ask user to report the issue in pedantic warning msg.

46a2f9f

sklam added 4 - Waiting on reviewer Waiting for reviewer to respond to author and removed 4 - Waiting on author Waiting for author to respond to review labels Jul 22, 2021

stuartarchibald reviewed Jul 22, 2021

View reviewed changes

Replace "error" with "warning"

edc82f7

sklam added 4 - Waiting on author Waiting for author to respond to review and removed 4 - Waiting on reviewer Waiting for reviewer to respond to author 4 - Waiting on author Waiting for author to respond to review labels Jul 22, 2021

sklam added the 4 - Waiting on reviewer Waiting for reviewer to respond to author label Jul 22, 2021

esc requested changes Jul 29, 2021

View reviewed changes

sklam and others added 2 commits July 29, 2021 11:45

Update numba/core/errors.py

d660e36

Co-authored-by: esc <esc@users.noreply.github.com>

Add comments to explain the dummy function in test

3d6fb07

sklam requested a review from esc July 29, 2021 16:50

esc reviewed Jul 30, 2021

View reviewed changes

esc approved these changes Jul 30, 2021

View reviewed changes

sklam mentioned this pull request Jul 30, 2021

Update URL in error message to use issue templates #7261

Closed

esc added 5 - Ready to merge Review and testing done, is ready to merge and removed 4 - Waiting on reviewer Waiting for reviewer to respond to author labels Jul 30, 2021

sklam merged commit 01dc3b4 into numba:master Jul 30, 2021

sklam deleted the fix/iss7217 branch July 30, 2021 19:30

sklam added a commit to sklam/numba that referenced this pull request Aug 4, 2021

Merge pull request numba#7223 from sklam/fix/iss7217

af36340

Replace assertion errors on IR assumption violation

sklam mentioned this pull request Nov 19, 2021

Fixes issue reported under #7217 #7228

Closed

	r"variable name '[a-z]' not in scope",
	r"variable '[a-z]' is not in scope",

	# pass when run with `error.NumbaPedanicWarning`s raised as
	# pass when run with `error.NumbaPedanticWarning`s raised as



		pedantic_warning_info = """
		This error came from an internal pedantic check. Please report the error message

	This error came from an internal pedantic check. Please report the error message
	This warning came from an internal pedantic check. Please report the warning message

Conversation

sklam commented Jul 20, 2021

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

stuartarchibald commented Jul 21, 2021

Uh oh!

stuartarchibald left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

stuartarchibald left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sklam commented Jul 21, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

stuartarchibald commented Jul 21, 2021

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

stuartarchibald Jul 22, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sklam commented Jul 27, 2021

Uh oh!

esc left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

esc left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

sklam commented Jul 21, 2021 •

edited

Loading

stuartarchibald Jul 22, 2021 •

edited

Loading