Fix for Issue 77 #78

lightsighter · 2021-10-22T20:45:22Z

Add support for a nice error message when users try to use legate outside of legion_python

…de of legion_python

manopapad · 2021-10-22T22:08:07Z

legate/core/__init__.py

+    using_legion_python = False
+if not using_legion_python:
+    raise RuntimeError(
+        'All Legate programs must be run with a legion_python interperter. We recommend that you use the Legate driver script "bin/legate" found in the installation directory to launch Legate programs as it provides easy-to-use flags for invoking legion_python. You can see options for using the driver script with "bin/legate --help". You can also invoke legion_python directly. Use "bin/legate --verbose ..." to see some examples of how to call legion_python directly.'


Our pre-commit hooks didn't complain about this long string! @marcinz @magnatelee you guys wrote the pre-commit config, can you check if it can be made to catch this?

manopapad · 2021-10-22T22:46:06Z

I changed the check. The original check wasn't actually working:

~/legate.numpy> PYTHONPATH=~/legate.core/install/lib/python3.8/site-packages/ python examples/gemm.py
Traceback (most recent call last):
  File "examples/gemm.py", line 26, in <module>
    import cunumeric as np
  File "/Users/mpapadakis/legate.core/install/lib/python3.8/site-packages/cunumeric/__init__.py", line 29, in <module>
    from cunumeric import linalg, random
  File "/Users/mpapadakis/legate.core/install/lib/python3.8/site-packages/cunumeric/linalg/__init__.py", line 19, in <module>
    from cunumeric.linalg.linalg import *
  File "/Users/mpapadakis/legate.core/install/lib/python3.8/site-packages/cunumeric/linalg/linalg.py", line 18, in <module>
    from cunumeric.array import ndarray
  File "/Users/mpapadakis/legate.core/install/lib/python3.8/site-packages/cunumeric/array.py", line 25, in <module>
    from legate.core import Array
  File "/Users/mpapadakis/legate.core/install/lib/python3.8/site-packages/legate/core/__init__.py", line 37, in <module>
    from legate.core.context import ResourceConfig
  File "/Users/mpapadakis/legate.core/install/lib/python3.8/site-packages/legate/core/context.py", line 18, in <module>
    from .legion import Future, legion
  File "/Users/mpapadakis/legate.core/install/lib/python3.8/site-packages/legate/core/legion.py", line 29, in <module>
    assert "LEGATE_MAX_DIM" in os.environ
AssertionError

Now we get:

~/legate.numpy> PYTHONPATH=~/legate.core/install/lib/python3.8/site-packages/ python examples/gemm.py
Traceback (most recent call last):
  File "examples/gemm.py", line 26, in <module>
    import cunumeric as np
  File "/Users/mpapadakis/legate.core/install/lib/python3.8/site-packages/cunumeric/__init__.py", line 29, in <module>
    from cunumeric import linalg, random
  File "/Users/mpapadakis/legate.core/install/lib/python3.8/site-packages/cunumeric/linalg/__init__.py", line 19, in <module>
    from cunumeric.linalg.linalg import *
  File "/Users/mpapadakis/legate.core/install/lib/python3.8/site-packages/cunumeric/linalg/linalg.py", line 18, in <module>
    from cunumeric.array import ndarray
  File "/Users/mpapadakis/legate.core/install/lib/python3.8/site-packages/cunumeric/array.py", line 25, in <module>
    from legate.core import Array
  File "/Users/mpapadakis/legate.core/install/lib/python3.8/site-packages/legate/core/__init__.py", line 22, in <module>
    raise RuntimeError(
RuntimeError: All Legate programs must be run with a legion_python interperter. We recommend that you use the Legate driver script "bin/legate" found in the installation directory to launch Legate programs as it provides easy-to-use flags for invoking legion_python. You can see options for using the driver script with "bin/legate --help". You can also invoke legion_python directly. Use "bin/legate --verbose ..." to see some examples of how to call legion_python directly.

lightsighter · 2021-10-22T22:54:53Z

legate/core/__init__.py

-except ModuleNotFoundError:
-    using_legion_python = False
-if not using_legion_python:
+if "LEGATE_MAX_DIM" not in os.environ:


I don't think this is sound. This depends on an environment variable which can be set manually by the user, even if legion_python is not being used. My original check was working. I even tested it. What were you seeing when it "wasn't working".

Testing for the presence of module legion_cffi will only catch the case where the user tries to use legate directly from the source tree without installing. If the user runs the install script (or uses a pre-compiled package) then legion_cffi will be present and found by both the legion_python interpreter and the regular python interpreter. You can see above that running python examples/gemm.py with the legate install dir added to PYTHONPATH (which I think is what a user is likely to do) bypasses the original check and asserts inside legate/core/legion.py.

This depends on an environment variable which can be set manually by the user, even if legion_python is not being used.

This is true, but I find it very unlikely that a user would have this particular variable set in their environment before ever running Legate. The most likely way that they will learn about this variable is if they try to run a Legate script and they get an error, and with this check we made sure that they don't just see an assertion failure like before, but a detailed error message on what to do.

Ok, I'll find a different way, but we're not going to rely on an environment variable for the test.

As you prefer. IMHO we don't need to go crazy on this, it's not critical that we catch every edge case.

…n python interpreter

lightsighter · 2021-10-25T09:09:41Z

I've pushed another attempt at addressing this.

lightsighter · 2021-10-25T09:09:56Z

I've pushed another attempt at addressing this.

manopapad · 2021-10-25T21:02:55Z

Now I get

~/cunumeric> PYTHONPATH=~/legate.core/install/lib/python3.8/site-packages/ python a.py
Traceback (most recent call last):
  File "a.py", line 2, in <module>
    import cunumeric as lg
  File "/Users/mpapadakis/cunumeric/cunumeric/__init__.py", line 29, in <module>
    from cunumeric import linalg, random
  File "/Users/mpapadakis/cunumeric/cunumeric/linalg/__init__.py", line 19, in <module>
    from cunumeric.linalg.linalg import *
  File "/Users/mpapadakis/cunumeric/cunumeric/linalg/linalg.py", line 18, in <module>
    from cunumeric.array import ndarray
  File "/Users/mpapadakis/cunumeric/cunumeric/array.py", line 25, in <module>
    from legate.core import Array
  File "/Users/mpapadakis/legate.core/install/lib/python3.8/site-packages/legate/core/__init__.py", line 25, in <module>
    if legion.legion_runtime_has_context():
  File "/Users/mpapadakis/opt/miniconda3/envs/legate/lib/python3.8/site-packages/cffi/api.py", line 912, in __getattr__
    make_accessor(name)
  File "/Users/mpapadakis/opt/miniconda3/envs/legate/lib/python3.8/site-packages/cffi/api.py", line 908, in make_accessor
    accessors[name](name)
  File "/Users/mpapadakis/opt/miniconda3/envs/legate/lib/python3.8/site-packages/cffi/api.py", line 838, in accessor_function
    value = backendlib.load_function(BType, name)
AttributeError: function/symbol 'legion_runtime_has_context' not found in library '<None>': dlsym(RTLD_DEFAULT, legion_runtime_has_context): symbol not found

lightsighter · 2021-10-25T21:35:21Z

@manopapad You need to rebuild with the most recent control replication.

manopapad · 2021-10-25T22:29:11Z

I nuked everything and rebuilt under latest control replication (commit a6673700d) and the behavior remains the same:

~/cunumeric> PYTHONPATH=~/legate.core/install/lib/python3.8/site-packages/ python examples/gemm.py
Traceback (most recent call last):
  File "examples/gemm.py", line 26, in <module>
    import cunumeric as np
  File "/Users/mpapadakis/legate.core/install/lib/python3.8/site-packages/cunumeric/__init__.py", line 29, in <module>
    from cunumeric import linalg, random
  File "/Users/mpapadakis/legate.core/install/lib/python3.8/site-packages/cunumeric/linalg/__init__.py", line 19, in <module>
    from cunumeric.linalg.linalg import *
  File "/Users/mpapadakis/legate.core/install/lib/python3.8/site-packages/cunumeric/linalg/linalg.py", line 18, in <module>
    from cunumeric.array import ndarray
  File "/Users/mpapadakis/legate.core/install/lib/python3.8/site-packages/cunumeric/array.py", line 25, in <module>
    from legate.core import Array
  File "/Users/mpapadakis/legate.core/install/lib/python3.8/site-packages/legate/core/__init__.py", line 25, in <module>
    if legion.legion_runtime_has_context():
  File "/Users/mpapadakis/opt/miniconda3/envs/legate/lib/python3.8/site-packages/cffi/api.py", line 912, in __getattr__
    make_accessor(name)
  File "/Users/mpapadakis/opt/miniconda3/envs/legate/lib/python3.8/site-packages/cffi/api.py", line 908, in make_accessor
    accessors[name](name)
  File "/Users/mpapadakis/opt/miniconda3/envs/legate/lib/python3.8/site-packages/cffi/api.py", line 838, in accessor_function
    value = backendlib.load_function(BType, name)
AttributeError: function/symbol 'legion_runtime_has_context' not found in library '<None>': dlsym(RTLD_DEFAULT, legion_runtime_has_context): symbol not found

magnatelee · 2021-10-25T22:35:02Z

I think AttributeError being raised here is telling that the core isn't running on the legion python launcher.

lightsighter · 2021-10-25T22:48:05Z

An AttributeError means that the library was loaded and that the symbol was missing when FFI parsed it, which would only happen if it wasn't build with that symbol, e.g. it was stale. Can you confirm that you can see the function name in legion_c.h?

magnatelee · 2021-10-25T22:58:55Z

@lightsighter well, legion_cffi only dlopens the current process (https://github.com/StanfordLegion/legion/blob/control_replication/bindings/python/legion_cffi.py.in#L30) and does not load the binding library by itself, so unless the process has already loaded the binding library, the interpreter won't be able to find that symbol. In other words, it's perfectly fine for any python interpreter to cdef the legion header without loading the binary, as long as the interpreter doesn't actually call any of the functions. That's why I think AttributeError is a good enough evidence. (Of course, some wild users may try to dlopen the binding library, but I don't think we need to worry about them.) Your hypothesis only makes sense if the process somehow managed to load the binding library but the library was missing that symbol, which I don't think is what's happening here.

manopapad · 2021-10-25T23:07:45Z

The symbol is in the header file and the .dylib file (although with an underscore, which might be a Mac thing, as all other C symbols seem to also start with an underscore)

~/legate.core/install> grep has_context include/legion/legion_c.h
   * @see Legion::Runtime::has_context()
  legion_runtime_has_context(void);
~/legate.core/install> nm lib/liblegion.dylib | grep has_context
0000000000037730 T __ZN6Legion7Runtime11has_contextEv
00000000000a7090 T _legion_runtime_has_context

…f legion_python

lightsighter · 2021-10-26T00:40:07Z

Pull and try again.

manopapad · 2021-10-26T19:27:00Z

Works now!

magnatelee · 2021-11-03T08:07:58Z

@lightsighter @manopapad I'd like to merge this PR, unless you have any other comments.

lightsighter · 2021-11-03T09:34:20Z

As far as I'm concerned it is ready when you guys are.

manopapad · 2021-11-03T15:50:45Z

No issues from me

Add a nice error message for the case where users invoke legate outsi…

d7ec4e7

…de of legion_python

lightsighter requested a review from magnatelee October 22, 2021 20:45

magnatelee approved these changes Oct 22, 2021

View reviewed changes

manopapad reviewed Oct 22, 2021

View reviewed changes

manopapad added 2 commits October 22, 2021 15:27

Break up long string

2c02f35

Fix check

18ff828

lightsighter commented Oct 22, 2021

View reviewed changes

Another attempt at adding a check for when we are inside of the legio…

fd5184e

…n python interpreter

More fixes for detecting when we are not running inside an instance o…

88a6ea9

…f legion_python

magnatelee merged commit 02103ad into nv-legate:branch-21.10 Nov 3, 2021

magnatelee mentioned this pull request Nov 3, 2021

Interpreter check #77

Closed

elliottslaughter pushed a commit to elliottslaughter/legate that referenced this pull request Mar 26, 2025

Expand testcases for null type and binary type. (nv-legate#78)

d650d6c

Fix for Issue 77 #78

Fix for Issue 77 #78

Uh oh!

Conversation

lightsighter commented Oct 22, 2021

Uh oh!

manopapad Oct 22, 2021

Choose a reason for hiding this comment

Uh oh!

manopapad commented Oct 22, 2021

Uh oh!

lightsighter Oct 22, 2021

Choose a reason for hiding this comment

Uh oh!

manopapad Oct 22, 2021

Choose a reason for hiding this comment

Uh oh!

lightsighter Oct 23, 2021

Choose a reason for hiding this comment

Uh oh!

manopapad Oct 23, 2021

Choose a reason for hiding this comment

Uh oh!

lightsighter commented Oct 25, 2021

Uh oh!

lightsighter commented Oct 25, 2021

Uh oh!

manopapad commented Oct 25, 2021

Uh oh!

lightsighter commented Oct 25, 2021

Uh oh!

manopapad commented Oct 25, 2021

Uh oh!

magnatelee commented Oct 25, 2021

Uh oh!

lightsighter commented Oct 25, 2021

Uh oh!

magnatelee commented Oct 25, 2021

Uh oh!

manopapad commented Oct 25, 2021

Uh oh!

lightsighter commented Oct 26, 2021

Uh oh!

manopapad commented Oct 26, 2021

Uh oh!

magnatelee commented Nov 3, 2021

Uh oh!

lightsighter commented Nov 3, 2021

Uh oh!

manopapad commented Nov 3, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants