llama.swiftui: Fix a small bug

[ho2103]

fix continuing generating blank lines after getting EOT token or EOS token from LLM

• I have read the contributing guidelines
• Self-reported review complexity:
  • Low
  • Medium
  • High

[HanClinto]

I'm trying to understand why this issue would only happen in Swift, and not in the other parallel projects, and looking at those, it looks like the paradigm is usually checking n_cur <= n_len -- but the Swift library (prior to your change) checks n_cur < n_len.

I'm not able to run this Swift library very easily -- could you please try changing this line:

Suggested change

	while await !llamaContext.latest_llama_token_is_eog_or_reach_len{
	while await llamaContext.n_cur <= llamaContext.n_len {

If you do that, do you still see the problem?

The main reason I ask is that -- as much as possible -- I'd like to keep the logic for all of the various libraries more or less parallel to each other, and I'm leery of one-off solutions that are only implemented in one platform at a time.

[ho2103]

"If you do that, do you still see the problem?" --- You changed back to the bug.
The problem is that, the original code for swiftUI frontend haven't checked the circumstances when there is EOT/EOS, so frontend keeps printing blank lines.
In the desired outcome, When there is EOT/EOS, frontend needs to stop generating any blank lines.
So, my solution is, besides checking if reaching n_len, you also need to check if the latest token is EOT/EOS, that is the whole meaning of why I added variable "latest_llama_token_is_eog_or_reach_len".

FYI, I haven't looked each line of original LLAMA code, but I did my best to use fewest line to fix this stupid bug.

[ho2103]

Because current n_len is just 64, you are not going to see the problem. If you change the n_len bigger, like 1024, you might find it keeps generating blank lines even after EOT/EOS

[HanClinto]

You changed back to the bug.

The original was <, I changed it to <= -- is there any difference in behavior if you do that?

[HanClinto]

FYI, I haven't looked each line of original LLAMA code, but I did my best to use fewest line to fix this stupid bug.

And yeah, don't feel bad -- it's too large of a project for anyone to have done that. I hope I'm not frustrating you -- I haven't looked at each line either. That's largely why I'm asking this. I'm not familiar with the Swift code, and am mainly trying to keep the various libraries from diverging too much. If a different portion of the library already solved this same problem in a different way, then in so far as we can keep the solutions more parallel, we should do that.

[HanClinto]

I see what you mean about needing EOT/EOS detection in addition to correct n_len handling.

What I haven't yet tracked down is -- how do the other portions of the code deal with EOT/EOS detection, and how can we keep the Swift library more parallel with those?

We don't need to read every line, but understanding how at least one other section of the code does it would be good. And if no other place does it, then maybe your fix should be replicated elsewhere.

[ggerganov]

Here is the check in simple example:

llama.cpp/examples/simple/simple.cpp

Lines 128 to 134 in d7fd29f

	// is it an end of generation?
	if (llama_token_is_eog(model, new_token_id) || n_cur == n_predict) {
	LOG_TEE("\n");
	break;
	}

The proposed change is good, but just change the name of the variable to is_done

[ggerganov]

Here is the check in simple example:

llama.cpp/examples/simple/simple.cpp

Lines 128 to 134 in d7fd29f

	// is it an end of generation?
	if (llama_token_is_eog(model, new_token_id) || n_cur == n_predict) {
	LOG_TEE("\n");
	break;
	}

The proposed change is good, but just change the name of the variable to is_done