FragPipe icon indicating copy to clipboard operation
FragPipe copied to clipboard

Spectral library generation part is using more threads that it is supposed to be

Open fcyu opened this issue 3 years ago • 6 comments

The EasyPQP convert command seems to be multithreading: each command is using all CPU cores. But the gen_con_spec_lib.py script also execute multiple convert commands in parallel, which makes the total thread equals #{CPU} * n, where n is the number of convert commands.

Best,

Fengchao

fcyu avatar Dec 21 '21 17:12 fcyu

This is a EasyPQP/pyOpenMS issue. EasyPQP uses pyOpenMS for parsing LCMS files, but you cannot control the number of threads used by the parser from pyOpenMS.

guoci avatar Dec 21 '21 17:12 guoci

If the convert command always use all CPUs, the gen_con_spec_lib.py should not run multiple convert command in parallel.

Also, can you create an issue to EasyPOP or OpenMS letting them add an option to set the number of threads.

Thanks,

Fengchao

fcyu avatar Dec 21 '21 17:12 fcyu

only the parsing is multithreaded, the rest of the code is single-threaded, so we might slow down the processing by doing that

guoci avatar Dec 21 '21 17:12 guoci

Then, please create an issue to EasyPQP or OpenMS to letting them add an option to set the number of threads. And then, set it to 1 in FragPipe. At any rate, we cannot have #{CPU} * n threads running. It will crash some HPC.

fcyu avatar Dec 21 '21 17:12 fcyu

submitted an issue: grosenberger/easypqp#76

guoci avatar Dec 21 '21 18:12 guoci

@fcyu update on the issue: https://github.com/grosenberger/easypqp/issues/76#issuecomment-1048932833 https://github.com/OpenMS/OpenMS/pull/5839

guoci avatar Feb 23 '22 15:02 guoci

This issue is pretty debilitating during the spectral library building process (which is otherwise great overall!). Is it possible to reconsider @fcyu suggestion to limit convert commands spawned by gen_con_spec_lib.py to one, at least until OpenMS releases a new version of pyOpenMS with the fix? EasyPQP generally runs pretty quick, and there does not seem to be too much heavy time-consuming computation in gen_con_spec_lib.py at a glance, so would it really slow down the processing that significantly if the convert processes were spawned serially? Any other parallelized code can remain the same. Just this snippet would need to be changed.

    subprocess.run([os.fspath(easypqp), '--version'], check=True)
    procs = []
    for i, e in enumerate(easypqp_convert_cmds):
        while sum(p.poll() is None for p in procs) >= nproc:
            time.sleep(1)
        procs.append(subprocess.Popen(e, cwd=os_fspath(output_directory), stdout=open(logs_directory / f'easypqp_convert_{i}.log', 'w'), stderr=subprocess.STDOUT))
        print(f'Executing {e}')

    for p in procs:
        p.wait()
    for i, p in enumerate(procs):
        if p.returncode != 0:
            print("EasyPQP convert error BEGIN")
            try:
                print(open(logs_directory / f'easypqp_convert_{i}.log').read(), end="")
            except OSError as e:
                print(e)
            print(f'exit status: {p.returncode}')
            print("EasyPQP convert error END")
    assert all(p.returncode == 0 for p in procs)

For example, I can convert the same results for a single pepXML and mzml file on my local machine using easypqp in 1.5 seconds, so for ~800 files that I am using to build a library, this convert process should take about ~20 minutes. However, the current convert processes spawned in parallel using gen_con_spec_lib.py on the server has taken/will take hours just to convert the files.

I could change the gen_con_spec_lib.py script I am using locally, but it could be worth considering implementing your own change, as I have seen this issue posted multiple times in different places (and I am running into it now). Thanks for the great software!

arnscott avatar Nov 03 '22 11:11 arnscott

Has been fixed in commit https://github.com/Nesvilab/FragPipe/commit/b85f869d35c3226fa21df6da88a29f324694c4b3

Best,

Fengchao

fcyu avatar Nov 30 '23 14:11 fcyu