search

charmplusplus / charm4py

Parallel Programming with Python and Charm++
https://charm4py.readthedocs.io
Apache License 2.0
289 stars 21 forks source link

Allow execution under analysis tools #207

Closed ZwFink closed 3 years ago

ZwFink commented 3 years ago

Fixes #206. Performance analysis and memory debugging tools such as Perf and Valgrind do not currently work with Charm4Py's charmrun.start. This patch enables the use of charmrun.start with these tools.

matthiasdiener commented 3 years ago

This might be interesting to test as part of CI. Also, could you provide more details about how this patch works? What is sys.executable in the case of running with (e.g.) perf?

ZwFink commented 3 years ago

Previously, charmrun.start would assume a Python file is provided for execution, and would prepend the absolute path to the Python interpreter to the command that is passed to charrmun. It uses sys.executable to find the Python interpreter.

The patch looks for an executable file passed to charmrun.start, and if it's a Python file, then it will add the path to the Python interpreter in the command passed to charmrun. Otherwise, it pass the command untouched to charmrun.

rbuch commented 3 years ago

Previously, charmrun.start would assume a Python file is provided for execution, and would prepend the absolute path to the Python interpreter to the command that is passed to charrmun. It uses sys.executable to find the Python interpreter.

The patch looks for an executable file passed to charmrun.start, and if it's a Python file, then it will add the path to the Python interpreter in the command passed to charmrun. Otherwise, it pass the command untouched to charmrun.

Can you add a comment to this effect to the source so that it's clear what it's supposed to do there?

ZwFink commented 3 years ago

This might be interesting to test as part of CI.

What is a good test case for CI? Presumably anything that eventually runs the target program should work, but we need it to run for macOS, Linux, and Windows. A simple bash script should suffice for the first two platforms, but Windows will require a bit of special handling.

evan-charmworks commented 3 years ago

cmd.exe -c <command line> might be sufficient on Windows.

ZwFink commented 3 years ago

LGTM, but are there cases where you may want to run a Python program that takes an argument that is executable? Perhaps this should only check if the first argument is a Python file or not to determine if it should insert sys.executable or not.

I'm not aware of any cases, but that certainly doesn't preclude their existence. Will update for this case.

ZwFink commented 3 years ago

In a previous comment I mentioned that testing this fix on Windows may require special handling, but the only tested platform is Linux. We probably want to test Mac/Windows, as we claim to support these systems in the documentation, but that's out of the scope of this fix.