isce-framework / s1-reader

Sentinel-1 reader
Apache License 2.0
27 stars 12 forks source link

Annotation class attrs #123

Open scottstanie opened 1 year ago

scottstanie commented 1 year ago

This is trying to prevent #122 from popping up sporadically. This is changing the classmethods in the annotation module to return instances: https://stackoverflow.com/a/14605349/4174466 All functions that are decorated with @classmethod are supposed to be alternate constructors (in addition to __init__); that is, they're meant to create a new instance. Right now, they have been changing "class attributes", which are shared across all instances of that class, and they are returning the class itself.

Why this is a problem If we kick of many instance of COMPASS in different threads, each one that reads bursts instantiates these annotation classes. they are all changing the same class attribute, so it's a data race where difference function calls to .from_et can clobber each other.

(example to show what this looks like)

```python In [11]: from concurrent.futures import ThreadPoolExecutor In [12]: import time, random In [13]: class A: ...: class_thing = 0 ...: @classmethod ...: def from_x(cls, x): ...: cls.class_thing = x ...: # pretend we're doing some other parsing here in the construction ...: time.sleep(random.random()) ...: # This is supposed to print the same thing twice: ...: print(f"{x}, {cls.class_thing}") ...: return cls() In [14]: a1 = A.from_x(3) 3, 3 In [15]: a2 = A.from_x(2) 2, 2 ``` if you try to do this method in many threads: ```python In [24]: tpe = ThreadPoolExecutor(max_workers=20) In [26]: alist = list(tpe.map(A.from_x, range(20))) 3, 19 18, 19 16, 19 7, 19 2, 19 8, 19 14, 19 6, 19 9, 19 12, 19 0, 19 11, 19 19, 19 5, 19 15, 19 4, 19 1, 19 10, 19 17, 19 13, 19 ```