Chapel: Should `--library-python`-generated modules automatically handle setup/cleanup?

[ben-albrecht]

When using a python-wrapped Chapel library (via --library-python), the python user must explicitly make setup/teardown calls for the runtime:

# Chapel library
import Foo

Foo.chpl_setup()
# do stuff
Foo.chpl_teardown()

I tend to attach the following code to all of my generated python modules (usually in the __init__.py), to avoid having to do this manually:

# Chapel library -- assuming package is built as Foo/Foo.cpython-*.so
from Foo import Foo

# From Python standard library
import atexit

# Register cleanup function to execute upon exiting program
atexit.register(Foo.chpl_cleanup)

Foo.chpl_setup()

I would guess that most users would prefer to have setup/cleanup of the Chapel runtime handled for them automatically.

Should we generate automatic setup/cleanup code in the python modules by default?

If we were to generate an __init__.py by default (suggested in https://github.com/chapel-lang/chapel/issues/13880), then we could place this code into the __init__.py, so that a user could easily opt out of it by removing the generated python code.

[lydia-duncan]

The main issue with that is that the setup function for multilocale libraries requires a numlocales argument - making the setup happen automatically would hard code that setting

[lydia-duncan]

I think it would be great to handle cleanup automatically

[ben-albrecht]

The main issue with that is that the setup function for multilocale libraries requires a numlocales argument - making the setup happen automatically would hard code that setting

That's a good point I hadn't considered. It might be nice when compiling with CHPL_COMM=none, but not sure having different generated python code depending on CHPL_COMM is worth it.

I think it would be great to handle cleanup automatically

Sounds good.

[lydia-duncan]

It might be nice when compiling with CHPL_COMM=none, but not sure having different generated python code depending on CHPL_COMM is worth it.

Yeah, I worry that would be confusing, although arguably no more confusing that requiring the numlocales argument at execution time.

This might also be complicated by exporting module level variables when we get to that, so I would probably wait until we have that story ironed out before considering how to handle initialization (and that's probably a bit down the road)

[ben-albrecht]

This might also be complicated by exporting module level variables when we get to that, so I would probably wait until we have that story ironed out before considering how to handle initialization (and that's probably a bit down the road)

Indeed. I think we can consider this feature request "do the cleanup automatically" for now.

[ben-albrecht]

It might be nice when compiling with CHPL_COMM=none, but not sure having different generated python code depending on CHPL_COMM is worth it.

Yeah, I worry that would be confusing, although arguably no more confusing that requiring the numlocales argument at execution time.

Chatting with @bradcray about this today, he suggested (A) This wouldn't be too different from the current differing behavior between CHPL_COMM=none not requiring any arguments and CHPL_COMM!=none requiring -nl <numLocales> for Chapel programs, and (B) Confusion could be avoided by a sufficiently helpful error message, e.g.

import someMultilocaleChapelLib as lib
lib.foo()
Error: Multi-locale Chapel requires a setup call specifying the number of locales, try:
         lib.chpl_setup(<numLocales>)

This reminds me - I need to open an issue for the current error message:

error: memory routine called before the memory layer is initialized

It should probably suggest a chpl_setup() call as well,

[dlongnecke-cray]

@ben-albrecht Scrolling through, did you ever make an issue for a "more helpful chpl.setup error message"?

[ben-albrecht]

Scrolling through, did you ever make an issue for a "more helpful chpl.setup error message"?

Just opened it - thanks for the reminder: https://github.com/chapel-lang/chapel/issues/15465

[dlongnecke-cray]

Is our conclusion here that we're only going to have the __init__.py do cleanup, for now?

[lydia-duncan]

I think so. It doesn't look like I've mentioned it here, but we may want to handle config variables by providing arguments to the setup function, so I wouldn't necessarily bundle them together

[ben-albrecht]

Yeah, let's consider the scope of this issue to be generating the following in an __init__.py:

"""__init__.py generated by Chapel compiler"""

import atexit

from <library-directory>.<chapel-module-name> import *

# Register cleanup function to execute upon exiting program
atexit.register(chpl_cleanup)

Note: I modified my original example such that this follows python standards better (based on pylint). This is dependent on #13880, which adds the __init__.py and the default import statement to begin with.

I will create a separate issue to track discussion/implementation of the automated setup support (https://github.com/chapel-lang/chapel/issues/13882#issuecomment-567648635)

[dlongnecke-cray]

Thanks Ben! I would hate to have to make another PR because the linter was complaining.

[dlongnecke-cray]

What would be a good way to test if the cleanup function is being called at exit? Should I find some way to probe atexit handlers? Or add a debug flag to chpl_setup? I'm currently using a print statement in chpl_cleanup right now but that's obviously only temporary...

[lydia-duncan]

A debug flag would work. I think we also have module deinitialization functions? You could add a printing line to one of those

[dlongnecke-cray]

Ok. If we add a debug flag to chpl_setup as a hidden last argument then I can add a test.