dstndstn / astrometry.net

Astrometry.net -- automatic recognition of astronomical images
http://astrometry.net
Other
650 stars 184 forks source link

Avoid mutating list while iterating. #265

Closed mbmccoy closed 1 year ago

mbmccoy commented 1 year ago

Iterates over a copy of subresults and jobresults so .remove calls on the lists do not mutate the list we are iterating over.

This change arises from debugging why the number of jobs processing was consistently lower than the number of processing threads. In short, it seems that modifying the jobresults list (jobresults.remove(...)) caused the list to consistently overestimate the number of remaining jobs. In my case, I was using 12 threads and was always reporting Still waiting for 8 Jobs each loop.

This seems to occur because of the mutation of jobsresults list. While the behavior of .remove while iterating through a list is undefined in python, running on my local machine demonstrates that this is likely to cause problems:

>>> x = list(range(12))
>>> for ii, v in enumerate(x):
...     print(ii, v)
...     x.remove(v)
... 
0 0
1 2
2 4
3 6
4 8
5 10
>>> x
[1, 3, 5, 7, 9, 11]

Tests on a running instance also suggest that the patch above fixes the issue.

dstndstn commented 1 year ago

Oh, this is great, thank you!