Add decode stream

[Virviil]

Bump a version of tokenizers

Bring new functionality. Mostly port from https://github.com/huggingface/tokenizers/pull/1678/files - here.

[josevalim]

I would likely do :done for when there is nothing more to decode.

[Virviil]

There is a problem here. While it looks like it's stream process, it's actually not. It's not exhausted. One can call
step(12345678) and then call step(0) - it's valid from original library perspective. Or one can call step(n) multiple times (this is even not idempotent).

https://docs.rs/tokenizers/0.21.1/tokenizers/tokenizer/struct.DecodeStream.html

use tokenizers::Tokenizer;
let tokenizer = Tokenizer::from_file("data/roberta.json").unwrap();

let mut decode_stream = tokenizer.decode_stream(false);
assert_eq!(decode_stream.step(713).unwrap(), Some("This".to_string()));
assert_eq!(decode_stream.step(16).unwrap(), Some(" is".to_string()));
assert_eq!(decode_stream.step(41).unwrap(), Some(" an".to_string()));
assert_eq!(
    decode_stream.step(1246).unwrap(),
    Some(" example".to_string())
);

From this perspective, :done looks inappropriate for having semantics of "last action", while it's not

[josevalim]

Maybe :out_of_range?

[Virviil]

Well

Enum.at/2 returns nil for out of range.
Erlang's libs (like array) raise argumentError.

I've never seen :out_of_range anywhere across ecosystem, so may be it will not work good.

[josevalim]

Enum.at does not wrap it in a tuple though. Enum.fetch, which returns {:ok, _} or :error, is a better example but there are only two states. My understanding is that this code has three states:

1. In range
2. Out of range
3. Error when decoding

So you need three states accordingly. I don't mind what we call it but I'd say the three states should be distinct.

[Virviil]

Switched to :out_of_range

[josevalim]

Suggested change

	case Tokenizers.Native.decoder_stream_step(decode_stream, tokenizer, id) do
	{:ok, result} -> {:ok, result}
	{:error, reason} -> {:error, reason}
	end
	Tokenizers.Native.decoder_stream_step(decode_stream, tokenizer, id)

[josevalim]

Is it actually efficient going step after step by giving specific indexes? Or does the upstream code works better by calling something .next? Is this something we should benchmark before?

[Virviil]

In this PR I translated API as is. Some abstraction can be definitely written on top of it using Stream, but it seems to be a part of separate effort.

Decoding large chunks this way looks better for BEAM because control is given back from NIF. There is a possibility to write decoding without dirty, but i'm not sure somebody will invest into it.

[josevalim]

Got it! Thank you!

[josevalim]

Thank you, I have dropped some comments. :)

[Virviil]

@jonatanklosko Can you review please?

[jonatanklosko]

If each step involves cloning, it may result in bad performance, even compared to decoding at once.

This may actually be a good reason to make the public API be a more high-level stream. For example, DecodeStream.new(...) can return %DecodeStream{} and it would implement Enumerable API, where the step NIF is called internally and we don't need to clone multiple times, since intermediate resources are not exposed to the user.

Thoughts?

[jonatanklosko]

Gah, the ids need to be an input, so it's not so much of Enumerable, but more like Stream.transform on top of a stream of ids. But I can see such API being cumbersome where the user doesn't want to block :<

[jonatanklosko]

@Virviil Actually, does ds.step mutate the tokenizer itself? Maybe there's no need for that particular clone in the first place.

[Virviil]

@jonatanklosko It was effectively possible to remove cloning.
Check 703f0e7 for diff.

[jonatanklosko]

It was effectively possible to remove cloning.

@Virviil great!