worker_helper: use sys.executable to run subproc

[albertz]

Currently we have:

        # Make sure Sisyphus is called again with the same python executable.
        # Adding the executable in front of the call could cause problems with the worker_wrapper
        if shutil.which(os.path.basename(sys.executable)) != sys.executable:
            os.environ["PATH"] = os.path.dirname(sys.executable) + ":" + os.environ["PATH"]
...
        call = sys.argv
...
        subprocess.check_call(call, stdout=logfile, stderr=logfile)

This failed for me:

    line: subprocess.check_call(call, stdout=logfile, stderr=logfile)
    locals:
      subprocess = <global> <module 'subprocess' from '/work/tools/users/zeyer/linuxbrew/opt/[email protected]/lib/python3.11/subprocess.py'>
      subprocess.check_call = <global> <function check_call at 0x7f67aed9f600>
      call = <local> ['sis', 'worker', '--engine', 'short', 'work/i6_core/returnn/training/ReturnnTrainingJob.Mw6ETRkehAUq', 'create_files', '1'], len = 7
      stdout = <not found>

Because I started Sisyphus this way:

/work/tools/users/zeyer/py-envs/py3.11-torch2.1/bin/python3.11 sis m recipe/i6_experiments/users/zeyer/experiments/exp2023_04_25_rf/conformer_import_moh_att_2023_06_30.py

Where sis is a file in the current directory. It is not a file anywhere in PATH. So that's why running sis m ... directly does not work.

A workaround is to start Sisyphus this way:

/work/tools/users/zeyer/py-envs/py3.11-torch2.1/bin/python3.11 ./sis m recipe/i6_experiments/users/zeyer/experiments/exp2023_04_25_rf/conformer_import_moh_att_2023_06_30.py

Then call becomes ["./sis", ...], which works. (Even though there could be cases where this would be wrong as well, as this hacking of PATH is not always correct. E.g. the Sis executable uses the shebang #!/usr/bin/env python3, and python3 might still be a different Python executable and environment.)

Anyway, I wonder, instead of having this PATH hack, why not using the sys.executable directly, as it is commonly done elsewhere? E.g. we have this:

#: Which command should be called to start sisyphus, can be used to replace the python binary
SIS_COMMAND = [sys.executable, sys.argv[0]]
# if this first argument is -m it's missing the module name
if sys.argv[0] == "-m":
    SIS_COMMAND += ["sisyphus"]

So, I don't really understand this comment "Adding the executable in front of the call could cause problems with the worker_wrapper". What problems? I think we anyway should do this to have it correct here and to avoid this hack.

[critias]

I looked into it again and don't know for sure anymore why I did it this way. I think there was some problem if a worker_wrapper call was defined with the default python binary e.g. adding singularity, but looking at the code I can't see the problem anymore and don't have singularity setup at hand to test it. I'm not against switching to sys.executable or SIS_COMMAND, but it would be good if someone could test it with singularity.

Overall I'm not very happy with this solution, but this was the only way I could find to really catch all program outputs and write them into a log file. Just overwriting sys.stderr and sys.stdout didn't work for me, I think subprograms still send everything to the regular stdout. I would more than happy to see a better solution for this. That would also avoid this fiddling around with the command line.