Replace psutil with multiprocessing

[alecandido]

psutil is only used to get the CPU count

• https://github.com/search?q=repo%3Aqiboteam%2Fqibo+%2F%5Cbpsutil%2F&type=code
• it is counting the logical units, which is the default option
  

  qibo/src/qibo/backends/tensorflow.py

  Line 200 in 1dd73f7

  self.nthreads = psutil.cpu_count(logical=True)

• the same could be achieved with multiprocessing.cpu_count() (which is standard library) - it would have not been possible for logical = False, but it is not what it's being used

#814

In [8]: psutil.cpu_count(logical=False)
Out[8]: 64

In [9]: psutil.cpu_count(logical=True) == multiprocessing.cpu_count()
Out[9]: True

Checklist:

• Reviewers confirm new code works as expected.
• Tests are passing.
• Coverage does not decrease.
• Documentation is updated.

[scarrazza]

Could you please double-check if this works in the cluster when selecting a specific number of cpus?

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (c1227d8) 100.00% compared to head (9ce8313) 100.00%.

Additional details and impacted files

@@            Coverage Diff            @@
##            master     #1119   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           65        65           
  Lines         9059      9058    -1     
=========================================
- Hits          9059      9058    -1     

Flag	Coverage Δ
unittests	100.00% <100.00%> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[alecandido]

Could you please double-check if this works in the cluster when selecting a specific number of cpus?

For sure. Is there a specific test that you want me to run?

In any case, the atomic change has been checked (as you can see from the IPython snippet above), and I've done it in maryah.
If that works, I don't imagine any way of failing during an actual run.

[scarrazza]

Psutils was introduced when we realized that the cpu count/affinity was not correct using other strategies when submitting jobs via slurm. Therefore, the best test is to send jobs by specifying the number of cpu cores (less and more than the default) and check if the output is correct.

[stavros11]

Psutils was introduced when we realized that the cpu count/affinity was not correct using other strategies when submitting jobs via slurm. Therefore, the best test is to send jobs by specifying the number of cpu cores (less and more than the default) and check if the output is correct.

What is the correct way of limiting the number of cores in slurm in order to test this? I have tried both srun and sbatch with different combinations of --ntasks, --cpus-per-task, --cores-per-socket but I always get the total number of threads (128) with both psutil and multiprocessing.

[scarrazza]

@stavros11, indeed these options are correct but I think the only way to set the proper cpu affinity is this.

[scarrazza]

Instead of guessing the number of available threads we might use tf.config.threading.get_inter_op_parallelism_threads or tf.config.threading.get_intra_op_parallelism_threads (please check which one is for single operator multi-threading) directly.

[alecandido]

In [2]: tf.config.get_inter_op_parallelism_threads
---------------------------------------------------------------------------
AttributeError                            Traceback (most recent call last)
Cell In[2], line 1
----> 1 tf.config.get_inter_op_parallelism_threads

AttributeError: module 'tensorflow._api.v2.config' has no attribute 'get_inter_op_parallelism_threads'

In [3]: tf.config.get_intra_op_parallelism_threads
---------------------------------------------------------------------------
AttributeError                            Traceback (most recent call last)
Cell In[3], line 1
----> 1 tf.config.get_intra_op_parallelism_threads

AttributeError: module 'tensorflow._api.v2.config' has no attribute 'get_intra_op_parallelism_threads'

[scarrazza]

https://www.tensorflow.org/api_docs/python/tf/config/threading/get_inter_op_parallelism_threads

[alecandido]

I believe they are now scoped in tf.config.threading:
https://www.tensorflow.org/api_docs/python/tf/config/threading/get_intra_op_parallelism_threads

[alecandido]

In [10]:  tf.config.threading.get_inter_op_parallelism_threads()
Out[10]: 0

In [11]:  tf.config.threading.get_intra_op_parallelism_threads()
Out[11]: 0

And if we do not set them, by default they are 0:

A value of 0 means the system picks an appropriate number.

[scarrazza]

Exactly, so ideally you could check for 0, if True assume tf is using all threads given by psutil affinity.

[alecandido]

And what should I do if not zero?

1. set to zero myself
2. ignore it (I will assume the user chose it manually)
3. raise an error
4. check if it's the same of the affinity (and then do something)
5. set to the affinity

My favorite option is 2., since it conflicts the least with user choices (definitely a case in which less is more).
But in that case I should do nothing in general (not even check it if it's zero, since I will ignore anyhow the outcome)

[scarrazza]

I think option 2 is reasonable, moreover qibo.set_threads consistently does not set threads when using the tf backend, but please double check if this value is not really needed anywhere.

[alecandido]

Thanks to @stavros11 I found it this interesting piece of Qibo:

qibo/src/qibo/backends/tensorflow.py

Lines 207 to 214 in c1227d8

	def set_threads(self, nthreads):
	log.warning(
	"`set_threads` is not supported by the tensorflow "
	"backend. Please use tensorflow's thread setters: "
	"`tf.config.threading.set_inter_op_parallelism_threads` "
	"or `tf.config.threading.set_intra_op_parallelism_threads` "
	"to switch the number of threads."
	)

So, I believe that it qibo.set_threads is not consistently doing its job. Especially for the TensorflowBackend. But I would not try to fix it in this PR.

[alecandido]

$ srun -w jubail poetry run python -c "import psutil; print(psutil.cpu_count(logical=False))"
128
$ srun -w jubail poetry run python -c "import psutil; print(psutil.cpu_count(logical=True))"
256
$ srun -w jubail poetry run python -c "import multiprocessing; print(multiprocessing.cpu_count())"
256
$ srun -w jubail --cpus-per-task 1 poetry run python -c "import psutil; print(psutil.cpu_count(logical=True))"
256
256
$ srun -w jubail --cpus-per-task 1 --ntasks 1 poetry run python -c "import psutil; print(psutil.cpu_count(logical=True))"
256

[scarrazza]

Indeed, the cluster uses affinity, therefore you should try: len(psutil.Process().cpu_affinity())

[alecandido]

Indeed, the cluster uses affinity, therefore you should try: len(psutil.Process().cpu_affinity())

In [6]: import psutil

In [7]: len(psutil.Process().cpu_affinity())
Out[7]: 2

This works, but it's not what we are currently using. Here, I just wanted to clean some unneeded dependency, not to introduce a new feature.

In case, I would open a dedicated issue.

[scarrazza]

Probably you have spotted a bug, in the sense that:

• the affinity is the only proper way to set the total number of available threads for OMP or thread pool library
• the tensorflow backend is the only backend which is not using affinity and instead randomly guessing the total number of threads instead of getting it from tf.config.threading directly.

[scarrazza]

LGTM, thanks.