TheCacophonyProject / classifier-pipeline

Exports tracked animals through thermal vision.
GNU General Public License v3.0
21 stars 14 forks source link

Remove constants in train.py #62

Closed rbtcollins closed 5 years ago

rbtcollins commented 5 years ago

Similar cleanup to elsewhere.

Bonus features, typos, grammar, docstrings and a lifted out function to permit DRY around the datasets.dat file location logic.

rbtcollins commented 5 years ago

Sigh, so this is failing on black - which I respect, but...


$ black --target-version=py36 build.py ml_tools/dataset.py train.py
Exception in thread QueueManagerThread:
Traceback (most recent call last):
  File "C:\Users\robertc\AppData\Local\Programs\Python\Python37\lib\threading.py", line 917, in _bootstrap_inner
    self.run()
  File "C:\Users\robertc\AppData\Local\Programs\Python\Python37\lib\threading.py", line 865, in run
    self._target(*self._args, **self._kwargs)
  File "C:\Users\robertc\AppData\Local\Programs\Python\Python37\lib\concurrent\futures\process.py", line 354, in _queue_management_worker
    ready = wait(readers + worker_sentinels)
  File "C:\Users\robertc\AppData\Local\Programs\Python\Python37\lib\multiprocessing\connection.py", line 868, in wait
    ready_handles = _exhaustive_wait(waithandle_to_obj.keys(), timeout)
  File "C:\Users\robertc\AppData\Local\Programs\Python\Python37\lib\multiprocessing\connection.py", line 800, in _exhaustive_wait
    res = _winapi.WaitForMultipleObjects(L, False, timeout)
ValueError: need at most 63 handles, got a sequence of length 65```

I suggest that rather than forcing every commit to be formatted, you have a robot reformat any misformatted state daily. This avoids issues with different developers having different versions of black etc (where avoids == 'don't spend dev time on this problem').,
rbtcollins commented 5 years ago

(Coming from https://github.com/ambv/black/issues/564 - my machine has indeed the magic 64 cores)

mjs commented 5 years ago

I've actually got in progress work which moves all the train.py config into the config file (i.e. the log file and all the hyper-parameters). You've got some other changes here which would be great to include so I'll merge and deal with the conflicts.

mjs commented 5 years ago

Bummer re hitting that problem with Black :)

I'm open to a bot doing the formatting at merge time. Are you aware of one that's already available?