-
Notifications
You must be signed in to change notification settings - Fork 216
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
Sampling, batch_all, non-zero, optimizer-flag #33
base: master
Are you sure you want to change the base?
Conversation
if args.loss_ignore_zero is True: | ||
nnz = tf.count_nonzero(losses, dtype=tf.float32) | ||
else: | ||
nnz = tf.reduce_sum(tf.to_float(tf.greater(losses, args.loss_ignore_zero or 1e-5))) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is the point of this supposed to be just for logging?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, actually. It's type-magic and can be a little obscure, hence why I still need to write documentation in the README :)
The else
case happens when loss_ignore_zero
is given an additional float argument, so one can call it as --loss_ignore_zero 1e-3
for example, in order to consider anything below 1e-3 to be counted as zero.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Read our paper, we explain them in there :) But really it's not a good time investment to play with that parameter.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am going to delete that comment because it makes no sense sorry. 😆 Currently have it printed and highlighted in front of me trying to get to grips!
help='Enable the super-mega-advanced top-secret sampling stabilizer.') | ||
|
||
parser.add_argument( | ||
'--loss_ignore_zero', default=False, const=True, nargs='?', type=common.positive_float, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ohh misread this to mean it can only be boolean. I am going to start playing with this then. 🍾
So this is a branch I had lying around for a while which adds a lot of things. I'm not merging it yet, as I still want to add these changes to the README and test it thoroughly, but here it is for others to try out.
It implements the trick I mentioned in #4 as well as some more variants we cover in the paper. If you give this a try, please give feedback here.