Bug with beam-size=1?

[mjpost]

In trying to get tests to passing with scoring (#538), I have turned up some weird behavior with scores output by Sockeye. Here are two commands using a transformer model built in the system tests. Notice:

• The invocations differ only in the beam size (1 or 2)
• --skip-topk is not enabled
• With beam size of 1, the scores output should be impossible, since Sockeye outputs negative logprobs.

Any ideas?

CC: @bricksdont

$ python3 -m sockeye.translate -i src --output-type translation_with_score --use-cpu -m model --beam-size 1 2> /dev/null | head
-10.556	7 5 2 7 3 6 5 4 7 7
-10.727	9 2 4 1 6 7 8 6 8
-12.788	8 6 8 7
-10.413	0 5 0 7 5 9 0 6 3 1
-10.731	7 9 2 6 8 5 0 6 5
-12.490	5 6 3 2
-inf	
-11.242	3 9 1 3 8 7
-15.759	2 1
-10.506	8 8 8 2 4 4 5 5 2 5
$ python3 -m sockeye.translate -i src --output-type translation_with_score --use-cpu -m model --beam-size 2 2> /dev/null | head
0.003	7 5 2 7 3 6 5 4 7 7
0.001	9 2 4 1 6 7 8 6 8
0.000	8 6 8 7
0.002	0 5 0 7 5 9 0 6 3 1
0.001	7 9 2 6 8 5 0 6 5
0.001	5 6 3 2
-inf	
0.001	3 9 1 3 8 7
0.001	2 1
0.002	8 8 8 2 4 4 5 5 2 5

[mjpost]

I will be digging into this, but thought I'd post an issue in case it looks familiar. Here are the training params (on a digits data set with 10,000 training points):

train_params = '--encoder transformer --decoder transformer --max-updates 4000 --num-layers 2 --transformer-attention-heads 4 --trans...l-learning-rate 0.001 --decode-and-evaluate 0 --label-smoothing 0.0 --optimized-metric perplexity --loss cross-entropy', 
translate_params = '--beam-size 1', 
translate_params_equiv = None, 
train_source_path = '/var/folders/mw/ks6b_m_x3916ybg0prddtg3r0000gn/T/test_seq_copy.9obml2kp/train.src'

[mjpost]

Okay, as I suspected, it seems that skip_softmax is hard-coded to True when using a beam size of 1. If this is the desired behavior, this issue can be closed. However, since it invalidates the scores that are output, it seems like perhaps it shouldn't be the default, but should instead be part of a package of optimizations used when the user wants things to be fast (we could document this in the forthcoming documentation under a "Fast Decoding" advanced features setting).

Edit: to add to this: the --skip-softmax CLI flag is weird—it only makes sense (and is only allowed) when beam size is 1, but it's also hardcoded to get turned on in that setting.

[bricksdont]

Yes - skip_softmax is enabled by default for beam size 1. Initially, this was a CLI option, but after some discussion we changed it to default:

PR Discussion: #519
Commit: 5c05dc3

I think we simply never considered scoring as a usecase so far. If there is consensus to change it back to optional for inference, I will open a PR.

[mjpost]

Okay. Perhaps it just isn't needed for scoring, particularly now that there will be a separate scoring module. I modified my sockeye.score tests to no longer try to reproduce the output of sockeye.translate when beam size is 1, so I have a fix on the scoring side of things.

[bricksdont]

Re your edit above:

Edit: to add to this: the --skip-softmax CLI flag is weird—it only makes sense (and is only allowed) when beam size is 1, but it's also hardcoded to get turned on in that setting.

--skip-softmax should not exist anymore. Did you see it somewhere in the current master? That would be a mishap on my side.

[mjpost]

Sorry, I meant --skip-topk.

[bricksdont]

No, skipping topk is not hardcoded to get turned on if beam size is 1, if --skip-topk is not used. Or, at least, that's not the intended behaviour :).

Relevant lines of code in inference.Translator:

sockeye/sockeye/inference.py

Line 1061 in 202f639

if self.skip_topk:

[mjpost]

Yes, I understand, skipping topk is enabled from the command line. However, skipping topk doesn't cause problems for scoring. Only skipping softmax does, and that is hardcoded.

[bricksdont]

Ok, sorry!

Edit: to add to this: the --skip-softmax CLI flag is weird—it only makes sense (and is only allowed) when beam size is 1, but it's also hardcoded to get turned on in that setting.

If you meant --skip-topk above, then I am confused about this. Can you please explain if it is still relevant?

[mjpost]

I was under the impression that --skip-topk was your command-line approach for turning off softmax. But it turned out they were two different things. Turning off softmax creates problems for comparing with the scorer, but toggling skipping topk doesn't.

(Why is topk-skipping an option? That just implements greedy decoding, so isn't using topk-skipping the same thing as using a beam size of 1, except with softmax? Is topk-skipping therefore just used to get the same outcome as a beam size of 1, by keeping softmax on?)

[bricksdont]

Skipping softmax and topk are not related, except that they both change throughput. By default, beam search uses utils.topk:

https://github.com/awslabs/sockeye/blob/master/sockeye/utils.py#L277

to retrieve the top k most probable words from the entire softmax (or logits, if softmax is skipped). For greedy decoding, k is simply 1. If --skip-topk, a different function is used for greedy decoding, utils.top1.

The CLI option exists because theoretically, top1 (= argmax or argmin) should be faster than topk. That's currently not the case, according to Kellen because of an MXnet bug.

[mjpost]

Sure, though they are conceptually related in that they both only make sense when the beam size is 1. This led to my two-part misunderstanding: (a) I wasn't aware of this MXNet bug, so it seemed strange that you'd pull out this special case, which should always be faster, and make it a command line argument, and (b) as a result, I thought --skip-topk was just a strange way to name the idea of turning off softmax.

Presumably, MXNet's topk should just do a max (or min) anyway if k==1, and we could do away with the Sockeye-side special case...

[bricksdont]

I see - yes, you are right that actually, if the implementation of MXNet argmin were fixed, it could also be used in MXNet topk. If one of them changes we should reconsider.

[fhieber]

Good point about the unnormalized scores with beam-size 1. As @bricksdont said, we didn't have a scoring use case so far, so we hadn't thought about this.
I think it would make sense to be more explicit about the skip-softmax optimization; at least lets add a logging statement at Translator construction time, that mentions that scores produced with beam-size 1 are sums of logits, not sums of logprobs.

[tdomhan]

good point regarding beam size 1. What if we changed sockeye.translate to only enable softmax skipping if the user does not specify an output handler, which uses the score?