neuroailab / tfutils

Utilities for working with tensorflow
MIT License
25 stars 8 forks source link

Make sure tfutils is python 3.5 compatible #102

Closed chengxuz closed 5 years ago

yamins81 commented 5 years ago

@chengxuz is this done? i can resolve this if not

chengxuz commented 5 years ago

Haven't examined this at all.

yamins81 commented 5 years ago

what's the right test suite for me to run to deal with this?

chengxuz commented 5 years ago

I think the most correct thing to do is to have the tests compatible in python 3.5 as well.

yamins81 commented 5 years ago

@chengxuz in doing this I'm finding the error

ModuleNotFoundError: No module named 'easy_batch_allreduce'

this has something to do with faster multi_gpu stuff I think. What's going on?

yamins81 commented 5 years ago

Here's the full error trace

Traceback (most recent call last): File "test_base.py", line 30, in import tfutils.base as base File "/home/yamins/tfutils/tfutils/base.py", line 10, in from tfutils.train import train_from_params File "/home/yamins/tfutils/tfutils/train.py", line 13, in import tfutils.utils as utils File "/home/yamins/tfutils/tfutils/utils.py", line 20, in from tfutils.multi_gpu.easy_variable_mgr import \ File "/home/yamins/tfutils/tfutils/multi_gpu/easy_variable_mgr.py", line 11, in import easy_batch_allreduce as batch_allreduce ModuleNotFoundError: No module named 'easy_batch_allreduce'

yamins81 commented 5 years ago

this is a genuine python3 compatibility issue am working on it

yamins81 commented 5 years ago

@chengxuz @damro (or anyone) does anyone object to my making six a requirement of tfutils? It enables python2-and-3 compatible checking of stuff like string types

yamins81 commented 5 years ago

@chengxuz in db_interface.py around line 131 there's an assignment:

total0 = None

Later, around line 144 we call

  range(total0)

This will error out in python2 or 3 unless total0 is reset in the for loop starting "for f in notindexfiles".

Is it guaranteed that this loop be executed (e.g. that notindexfiles be nonempty)?

The context is that I'm getting in error on line 144 saying that total0 is None, and thus getting a type error, but only in python3. What might be causing the for loop to be executed (thus resetting total0) in python2 but not in python3?

chengxuz commented 5 years ago

Hmm, I don't fully know the purpose or logic of this function. But based on my reading, it cannot be guaranteed that notindexfiles is nonempty.

yamins81 commented 5 years ago

@damro @chengxuz can you guys test my PR on this and merge if ready?