change DataIterator to work on get_example and get_batch and add methods... #40

mducoffe · 2015-03-06T20:45:51Z

... and batch attributes to Transformer object

…ods and batch attributes to Transformer object

mducoffe · 2015-03-06T20:48:48Z

Just to know if the code was okay. Also do I have to call get_epoch_iterator on every transformer class with example or batch stream for tests ? (Have also to correct indentation error)

bartvm · 2015-03-06T21:27:58Z

fuel/iterator.py

@@ -23,9 +23,15 @@ def __iter__(self):

    def __next__(self):
        if self.request_iterator is not None:
-            data = self.data_stream.get_data(next(self.request_iterator))
+            if self.data_stream.batch :


Mmmm, maybe it's better to move this logic to the Transformer base class. That way child classes can override it if necessary and don't need to duplicate methods if the implementation for batch/example is the same.

bartvm · 2015-03-06T21:42:01Z

There are a few transformers which can share the implementation of their get_example and get_batch method, so here's how I would propose doing it:

Implement a get_data method on the Transformer base class that calls get_example and get_batch (like the logic you implemented in the DataIterator now). Create two get_example and get_batch methods which raise a NotImplementedError when called
Transformers that have the same logic for batch and example just override the get_data method
Transformers for which the methods are different, or which only implement one, override get_example/get_batch instead.

Hope that was clear

…ds are possible

bartvm · 2015-03-09T20:47:51Z

fuel/transformers/__init__.py

-    def __init__(self, data_stream, mapping, add_sources=None):
-        super(Mapping, self).__init__(data_stream)
+    def __init__(self, data_stream, mapping, batch=False, add_sources=None):
+        super(Mapping, self).__init__(data_stream, batch)


So you can remove it here too.

bartvm · 2015-03-09T21:07:51Z

Seems like there's a merge conflict, so you'll need to rebase. Let me know if you need some help doing that!

…a_from_example, get_batch, swap Unpack and Batch, remove batch from the inputs of the classes derived from Transformer

…to CCW_Transformer Conflicts: fuel/transformers/__init__.py

mducoffe · 2015-03-12T17:49:42Z

@bartvm it is eventually done !

mducoffe · 2015-03-16T02:58:57Z

@bartvm do u know what is coverage test in the github tests ?

bartvm · 2015-03-16T03:12:38Z

Coverage is the number of lines of code that are run by unit tests divided by the total number of lines of code. If it decreases it means that this percentage dropped. In this case it's because you added 9 new lines of code to transformers/__init__.py, but only 7 of those are tested, so the coverage decreases slightly. There's no need to worry about this too much, it's mostly to remind people that their code needs to be tested, and if the drop is very large then it is a sign that tests need to be written. You can actually click on the details and see exactly which line is currently untested.

mducoffe · 2015-03-16T13:02:57Z

@bartvm is there any way to see which lines are not covered by the tests ? (sources are not available)

bartvm · 2015-03-16T13:06:23Z

Not sure, never seen that before! It might be because of the merge conflicts earlier. You might want to try rebasing?

bartvm · 2015-03-19T20:24:38Z

fuel/transformers/__init__.py

@@ -22,10 +22,14 @@ class Transformer(AbstractDataStream):
        this attribute. Use it to access data from the wrapped data stream
        by calling ``next(self.child_epoch_iterator)``.

+    batch : boolean


batch_input instead of batch, and there should be no new line between parameter descriptions.

bartvm · 2015-03-20T17:48:07Z

Wait, I just realised you're working in #45 now, so I'll close this one then?

mducoffe · 2015-03-20T17:49:19Z

yes please. And by the way thanks for the dropout on blocks, it is really
convenient to use with the computation graph

2015-03-20 13:48 GMT-04:00 Bart van Merriënboer notifications@github.com:

Wait, I just realised you're working in #45
#45 now, so I'll close this one then?

—
Reply to this email directly or view it on GitHub
#40 (comment).

change DataIterator to work on get_example and get_batch and add meth…

f669330

…ods and batch attributes to Transformer object

bartvm reviewed Mar 6, 2015
View reviewed changes

bartvm mentioned this pull request Mar 8, 2015

Transformers should have batch- and example-specific methods #27

Closed

Melanie Ducoffe added 2 commits March 9, 2015 09:19

add overide of get_example and get_batch or get_data if the two metho…

2653454

…ds are possible

add overide of get_example and get_batch or get_data if the two metho…

a49e5a6

…ds are possible

bartvm reviewed Mar 9, 2015
View reviewed changes

Melanie Ducoffe added 5 commits March 10, 2015 09:31

change get_example, get_data, batch into get_data_from_batch, get_dat…

0cc19c9

…a_from_example, get_batch, swap Unpack and Batch, remove batch from the inputs of the classes derived from Transformer

solve merge conflicts

579bd2c

Merge branch 'CCW_Transformer' of https://github.com/mducoffe/fuel in…

3c6e49f

…to CCW_Transformer Conflicts: fuel/transformers/__init__.py

pep8 correction

01adf80

end of correction

4f9d814

bartvm reviewed Mar 19, 2015
View reviewed changes

bartvm closed this Mar 20, 2015

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

change DataIterator to work on get_example and get_batch and add methods... #40

change DataIterator to work on get_example and get_batch and add methods... #40

mducoffe commented Mar 6, 2015

mducoffe commented Mar 6, 2015

bartvm Mar 6, 2015

bartvm commented Mar 6, 2015

bartvm Mar 9, 2015

bartvm commented Mar 9, 2015

mducoffe commented Mar 12, 2015

mducoffe commented Mar 16, 2015

bartvm commented Mar 16, 2015

mducoffe commented Mar 16, 2015

bartvm commented Mar 16, 2015

bartvm Mar 19, 2015

bartvm commented Mar 20, 2015

mducoffe commented Mar 20, 2015

change DataIterator to work on get_example and get_batch and add methods... #40

change DataIterator to work on get_example and get_batch and add methods... #40

Conversation

mducoffe commented Mar 6, 2015

mducoffe commented Mar 6, 2015

bartvm Mar 6, 2015

Choose a reason for hiding this comment

bartvm commented Mar 6, 2015

bartvm Mar 9, 2015

Choose a reason for hiding this comment

bartvm commented Mar 9, 2015

mducoffe commented Mar 12, 2015

mducoffe commented Mar 16, 2015

bartvm commented Mar 16, 2015

mducoffe commented Mar 16, 2015

bartvm commented Mar 16, 2015

bartvm Mar 19, 2015

Choose a reason for hiding this comment

bartvm commented Mar 20, 2015

mducoffe commented Mar 20, 2015