Skip to content

Fixes for float16 problems in the DLT#172

Merged
lamblin merged 5 commits intolisa-lab:masterfrom
abergeron:mixed
Oct 31, 2016
Merged

Fixes for float16 problems in the DLT#172
lamblin merged 5 commits intolisa-lab:masterfrom
abergeron:mixed

Conversation

@abergeron
Copy link
Copy Markdown
Contributor

No description provided.

avg_acceptance_rate = sharedX(target_acceptance_rate,
'avg_acceptance_rate')
s_rng = TT.shared_randomstreams.RandomStreams(seed)
s_rng = theano.sandbox.rng_mrg.MRG_RandomStreams(seed)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AttributeError: 'module' object has no attribute 'sandbox'

code/rbm.py Outdated
self.input * T.log(T.nnet.sigmoid(pre_sigmoid_nv)) +
(1 - self.input) * T.log(1 - T.nnet.sigmoid(pre_sigmoid_nv)),
self.input * T.log(T.nnet.sigmoid(pre_sigmoid_nv) + 0.001) +
(1 - self.input) * T.log(1 - T.nnet.sigmoid(pre_sigmoid_nv) + 0.001),
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would it make sence to add in theano a log_sigmoid like log_softmax?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I dropped that part since the root of the problem was elsewhere.

@abergeron abergeron changed the title Use MRG_RandomStreams instead for shared_randomstreams for GPU compat. Fixes for float16 problems in the DLT Oct 24, 2016
c.append(train_da(batch_index))

print('Training epoch %d, cost ' % epoch, numpy.mean(c))
print('Training epoch %d, cost ' % epoch, numpy.mean(c, dtype='float64'))
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you know what is going on about numpy.mean? if c are float16 output I checked that numpy output float16. In my tests, it seem the accumulator is in float16 or something like that. Do you know?

We would need to document that in Theano about float16. At least in this issue: Theano/Theano#2908. I let you modify it, in case you can add more information.

Should we special case float16 and make Theano always return at least float32 to help prevent that type of problems?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes the accumulator is float16 internally and overflows.

Do we have a page about float16 gotchas in Theano. This is the only place where I would see this sort of information.

I would oppose special-casing outputs in Theano, because the problem is easily resolved by the user and very visible.

@nouiz
Copy link
Copy Markdown
Member

nouiz commented Oct 28, 2016

I added the comment to the issue about float16:
Theano/Theano#2908

On Fri, Oct 28, 2016 at 6:34 PM, abergeron [email protected] wrote:

@abergeron commented on this pull request.

In code/dA.py #172
:

@@ -336,7 +336,7 @@ def test_dA(learning_rate=0.1, training_epochs=15,
for batch_index in range(n_train_batches):
c.append(train_da(batch_index))

  •    print('Training epoch %d, cost ' % epoch, numpy.mean(c))
    
  •    print('Training epoch %d, cost ' % epoch, numpy.mean(c, dtype='float64'))
    

Yes the accumulator is float16 internally and overflows.

Do we have a page about float16 gotchas in Theano. This is the only place
where I would see this sort of information.

I would oppose special-casing outputs in Theano, because the problem is
easily resolved by the user and very visible.


You are receiving this because you commented.
Reply to this email directly, view it on GitHub
#172, or mute the
thread
https://github.com/notifications/unsubscribe-auth/AALC--j0h9VH6yuqu5w46Vf5pAk9ZeMlks5q4nhogaJpZM4KM5eO
.

@nouiz
Copy link
Copy Markdown
Member

nouiz commented Oct 31, 2016

Is this ready to merge?

@lamblin lamblin merged commit f1c0587 into lisa-lab:master Oct 31, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants