Improve LISI multiprocessing specification

[mumichae]

LISI supports multiprocessing, however specifying the number of cores and whether to use multiprocessing is not well defined.
Here I address

1. remove multiprocessing key from lisi functions
2. rename nodes to n_cores
3. use n_cores=1 for running without parallelisation
4. use n_cores=1 as default for metrics wrapper functions

Bug fix included:

1. changed expected input of n_chunks for knn_graph.o to actually reflect the number of chunks (#chunks) created, NOT the index of the largest chunk (chunks-1)
2. compile knn_graph.cpp file on install with setup.py

[LuckyMD]

Overall this looks great, I like the simplification of the API. The chunk calculation is now no longer done... any reason why?

[mumichae]

From what I understood from the code, the number of splits is basically the same as the number of cpu cores - 1, so I simplified it as such.
To be exact, n_chunks == n_processes and n_splits == n_chunks - 1

[LuckyMD]

To be exact, n_chunks == n_processes and n_splits == n_chunks - 1

True, just got this as well. And you don't want to use the whole node if not specified otherwise?

[LuckyMD]

Looks good!

Only one thing: check if you still need to import multiprocessing if you removed the call to find how many cpus there are on the machine.

[mumichae]

I realised that we actually had a bug in the C code. When lisi_graph_py is called with 2 cores, the script will set the max chunk index to 0, although it should be 1. I fixed this and had to recompile the code.

Unfortunately, the last step failed on github actions likely because the g++ version I was using was not compatible with the one on github actions. For that, I also included the lisi code compilation into the setup.py. Might be a bit hacky, but that's the only thing that I managed to get to work.

@LuckyMD Would be great if I could have another review on this decision.

[LuckyMD]

I find this difficult to review as I didn't write the .cpp code. Maybe @mbuttner could take a look at this to okay it?

Additional tests look good

[LuckyMD]

I actually like the compilation in setup.py. No testing whether g++ is installed though, so it might create some strange errors.

[mbuttner]

I think that the LISI code looks much nicer now, thanks for adding the tests and cleaning up the multiprocessing part. Please add an error report when compilation does not work. Well done!

[mbuttner]

In the previous implementation of the multiprocessing, I have taken all but one CPU for the process to be a little less greedy. But when I checked my previous implementation, it is apparently not doing what the comment said but taking all CPUs. So please ignore.

[mumichae]

Oh right, I was adapting it to have an upper bound and initially used n_cpu-1, before realising that the code will only run in single core on github action, since the machine only has 2 cores. So the lower bound is redundant, will remove.

[mumichae]

Thanks @mbuttner for the review!

I updated the compilation call so that any error message will be printed, but the installation will continue. The output is only visible with pip install scib -v, but that's a pip thing.

Once everything runs through I'll merge and update scib to a new version.