Repository: mila-udem/fuel (python)

Issue:
Problems with using H5PyDataset after deserialization
The traceback is below. It seems that the first request that unpickled dataset gets is `next_epoch`, which calls `reset`, which calls `close`. `close`, however, requests the attribute `external_file_handle` bypassing the property. This way `load` is not called, property is not set, and `AttributeError` is raised.

Why are there so many places in `H5PyDataset` where `_external_file_handle`, not `external_file_handle` is used?

```
  File "/home/rizar/dist/fully-neural-lvsr/libs/blocks/bin/blocks-continue", line 15, in <module>
    continue_training(args.path)
  File "/home/rizar/dist/fully-neural-lvsr/libs/blocks/blocks/serialization.py", line 220, in continue_training
    main_loop.run()
  File "/home/rizar/dist/fully-neural-lvsr/libs/blocks/blocks/main_loop.py", line 196, in run
    reraise_as(e)
  File "/home/rizar/dist/fully-neural-lvsr/libs/blocks/blocks/utils/__init__.py", line 225, in reraise_as
    six.reraise(type(new_exc), new_exc, orig_exc_traceback)
  File "/home/rizar/dist/fully-neural-lvsr/libs/blocks/blocks/main_loop.py", line 182, in run
    while self._run_epoch():
  File "/home/rizar/dist/fully-neural-lvsr/libs/blocks/blocks/main_loop.py", line 225, in _run_epoch
    get_epoch_iterator(as_dict=True))
  File "/home/rizar/dist/fully-neural-lvsr/libs/fuel/fuel/transformers/__init__.py", line 64, in get_epoch_iterator
    self.child_epoch_iterator = self.data_stream.get_epoch_iterator()
  File "/home/rizar/dist/fully-neural-lvsr/libs/fuel/fuel/transformers/__init__.py", line 64, in get_epoch_iterator
    self.child_epoch_iterator = self.data_stream.get_epoch_iterator()
  File "/home/rizar/dist/fully-neural-lvsr/libs/fuel/fuel/transformers/__init__.py", line 64, in get_epoch_iterator
    self.child_epoch_iterator = self.data_stream.get_epoch_iterator()
  File "/home/rizar/dist/fully-neural-lvsr/libs/fuel/fuel/transformers/__init__.py", line 64, in get_epoch_iterator
    self.child_epoch_iterator = self.data_stream.get_epoch_iterator()
  File "/home/rizar/dist/fully-neural-lvsr/libs/fuel/fuel/transformers/__init__.py", line 246, in get_epoch_iterator
    super(Filter, self).get_epoch_iterator(**kwargs)
  File "/home/rizar/dist/fully-neural-lvsr/libs/fuel/fuel/transformers/__init__.py", line 64, in get_epoch_iterator
    self.child_epoch_iterator = self.data_stream.get_epoch_iterator()
  File "/home/rizar/dist/fully-neural-lvsr/libs/fuel/fuel/transformers/__init__.py", line 64, in get_epoch_iterator
    self.child_epoch_iterator = self.data_stream.get_epoch_iterator()
  File "/home/rizar/dist/fully-neural-lvsr/libs/fuel/fuel/transformers/__init__.py", line 64, in get_epoch_iterator
    self.child_epoch_iterator = self.data_stream.get_epoch_iterator()
  File "/home/rizar/dist/fully-neural-lvsr/libs/fuel/fuel/streams.py", line 137, in get_epoch_iterator
    self.next_epoch()
  File "/home/rizar/dist/fully-neural-lvsr/libs/fuel/fuel/streams.py", line 128, in next_epoch
    self.data_state = self.dataset.next_epoch(self.data_state)
  File "/home/rizar/dist/fully-neural-lvsr/libs/fuel/fuel/datasets/base.py", line 158, in next_epoch
    return self.reset(state)
  File "/home/rizar/dist/fully-neural-lvsr/libs/fuel/fuel/datasets/base.py", line 139, in reset
    self.close(state)
  File "/home/rizar/dist/fully-neural-lvsr/libs/fuel/fuel/datasets/hdf5.py", line 596, in close
    self._out_of_memory_close()
  File "/home/rizar/dist/fully-neural-lvsr/libs/fuel/fuel/datasets/hdf5.py", line 599, in _out_of_memory_close
    if not self._external_file_handle:
AttributeError: 'H5PYAudioDataset' object has no attribute '_external_file_handle'
```

Pull request description:
Fix deserialization issues in H5PYDataset
Fixes #215.

@rizar correctly identified that this was caused by `H5PYDataset` accessing `self._external_file_handle` rather than `self.external_file_handle` outside `H5PYDataset.load`.

Git blame points out this is my mistake, sorry about that!

I added this case to the serialization unit test so it's catched the next time.

Patch:
diff --git a/fuel/datasets/hdf5.py b/fuel/datasets/hdf5.py
index c32a9ca..fa5e877 100644
--- a/fuel/datasets/hdf5.py
+++ b/fuel/datasets/hdf5.py
@@ -581,7 +581,7 @@ class H5PYDataset(Dataset):
         return None if self.load_in_memory else self._out_of_memory_open()
 
     def _out_of_memory_open(self):
-        if not self._external_file_handle:
+        if not self.external_file_handle:
             if self.path not in self._file_handles:
                 handle = h5py.File(
                     name=self.path, mode="r", driver=self.driver)
@@ -593,7 +593,7 @@ class H5PYDataset(Dataset):
             self._out_of_memory_close()
 
     def _out_of_memory_close(self):
-        if not self._external_file_handle:
+        if not self.external_file_handle:
             self._ref_counts[self.path] -= 1
             if not self._ref_counts[self.path]:
                 del self._ref_counts[self.path]
@@ -602,8 +602,8 @@ class H5PYDataset(Dataset):
 
     @property
     def _file_handle(self):
-        if self._external_file_handle:
-            return self._external_file_handle
+        if self.external_file_handle:
+            return self.external_file_handle
         elif self.path in self._file_handles:
             return self._file_handles[self.path]
         else:


Test patch:
diff --git a/tests/test_hdf5.py b/tests/test_hdf5.py
index 3639666..aed8ed3 100644
--- a/tests/test_hdf5.py
+++ b/tests/test_hdf5.py
@@ -154,6 +154,10 @@ class TestH5PYDataset(object):
             h5file.attrs['split'] = H5PYDataset.create_split_array(split_dict)
             dataset = cPickle.loads(
                 cPickle.dumps(H5PYDataset(h5file, which_sets=('train',))))
+            # Make sure _out_of_memory_{open,close} accesses
+            # external_file_handle rather than _external_file_handle
+            dataset._out_of_memory_open()
+            dataset._out_of_memory_close()
             assert dataset.data_sources is None
         finally:
             os.remove('file.hdf5')