Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Device placement is logged by default #88

Open
kiudee opened this issue Feb 27, 2020 · 4 comments
Open

Device placement is logged by default #88

kiudee opened this issue Feb 27, 2020 · 4 comments
Labels
bug Something isn't working enhancement New feature or request Priority: Medium

Comments

@kiudee
Copy link
Owner

kiudee commented Feb 27, 2020

We have a utility function configure_numpy_keras which is used in some of the experiment scripts:

def configure_numpy_keras(seed=42):
tf.set_random_seed(seed)
os.environ["KERAS_BACKEND"] = "tensorflow"
devices = [x.name for x in device_lib.list_local_devices()]
logger = logging.getLogger("ConfigureKeras")
logger.info("Devices {}".format(devices))
n_gpus = len([x.name for x in device_lib.list_local_devices() if x.device_type == 'GPU'])
if n_gpus == 0:
config = tf.ConfigProto(intra_op_parallelism_threads=1, inter_op_parallelism_threads=1,
allow_soft_placement=True, log_device_placement=False,
device_count={'CPU': multiprocessing.cpu_count() - 2})
else:
config = tf.ConfigProto(allow_soft_placement=True,
log_device_placement=True, intra_op_parallelism_threads=2,
inter_op_parallelism_threads=2) # , gpu_options = gpu_options)
sess = tf.Session(config=config)
K.set_session(sess)
np.random.seed(seed)
logger.info("Number of GPUS {}".format(n_gpus))

It does the following:

  • Set random seeds
  • Sets the KERAS_BACKEND to Tensorflow
  • Checks the number of GPUs and sets the Tensorflow options accordingly
  • Creates a Tensorflow session for Keras to use

There are a few issues (and maybe more) with this:

  • Everything is set to hardcoded constants. Making it configurable is desirable.
  • log_device_placement is set to True, which can cause slowdowns due to logging and should be False by default.
  • It is not clear, if tensorflow_util.py is the correct location, if the function is only ever used in experiments.
  • It is not documented.
@kiudee kiudee added bug Something isn't working enhancement New feature or request Priority: Medium labels Feb 27, 2020
@timokau timokau self-assigned this May 29, 2020
@timokau
Copy link
Collaborator

timokau commented May 29, 2020

Since the experiments have been removed, there is only one remaining place this is used (the Visualize-NeuralNetwork notebook). Its probably best to just inline the relevant functionality there.

@timokau
Copy link
Collaborator

timokau commented May 29, 2020

I can't get the code in that notebook to run. First of all, it depends on pydot which we don't currently declare as a dependency. When one installs that, it fails with

Traceback (most recent call last):
  File "experiment.py", line 65, in <module>
    plot_model(plot_model(gor.model,to_file=model_path,show_shapes=True, rankdir='LR'))
  File "/nix/store/z7c1wcmcgf7qjp1m0bfysfk26m4gbaz4-python3-3.7.7-env/lib/python3.7/site-packages/keras/utils/vis_utils.py", line 240, in plot_model
    expand_nested, dpi)
  File "/nix/store/z7c1wcmcgf7qjp1m0bfysfk26m4gbaz4-python3-3.7.7-env/lib/python3.7/site-packages/keras/utils/vis_utils.py", line 99, in model_to_dot
    layers = model._layers
AttributeError: 'Image' object has no attribute '_layers'

Its somewhat unclear what the intention behind that notebook is in the first place. Maybe it should have been removed together with the experiments. What do you think @prithagupta @kiudee?

This is the notebook in question: https://github.com/kiudee/cs-ranking/blob/master/docs/notebooks/Visualize-NeuralNetwork.ipynb

@timokau timokau removed their assignment May 29, 2020
@prithagupta
Copy link
Collaborator

@timokau I made this notebook so that, we can have one notebook to visualize the network. But I don't think it is needed now that we have the tensor board and users can do that if they want it themselves. I think you can remove it.

@prithagupta
Copy link
Collaborator

@kiudee yes, I agree we need a better structure for the utils methods. If possible I would also like to replace the pymc3 with Theano backend to the pymc4 with TensorFlow backend or with botorch. It should be much faster and also should be easier to implement these models.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working enhancement New feature or request Priority: Medium
Projects
None yet
Development

No branches or pull requests

3 participants