Vraspar/phi 3 ios update #467
Conversation
| @@ -1,27 +1,124 @@ | |||
| // Copyright (c) Microsoft Corporation. All rights reserved. | |||
There was a problem hiding this comment.
should we keep the copyright notice?
| let totalTime = userInfo["totalTime"] as? Int, | ||
| let firstTokenTime = userInfo["firstTokenTime"] as? Int, | ||
| let tokenCount = userInfo["tokenCount"] as? Int { | ||
| stats = "Generated \(tokenCount) tokens in \(totalTime) ms. First token in \(firstTokenTime) ms." |
There was a problem hiding this comment.
could we also include something like this:
token generation rate (tokens/second) = (tokenCount - 1) * 1000 / (totalTimeInMs - firstTokenTimeInMs)
the token generation and prompt processing rates may be different, and it might be useful to get a sense of both.
There was a problem hiding this comment.
Would be nice to have prompt tokens/s as well given that and generation tokens/s are the usual metrics that gets compared. That would require something to return the number of tokens in the prompt (possibly the length of the input sequence post-tokenization) as IIRC it's not 1:1 with the number of words.
|
|
||
| auto sequences = OgaSequences::Create(); | ||
| tokenizer->Encode(prompt, *sequences); | ||
| typedef std::chrono::high_resolution_clock Clock; |
There was a problem hiding this comment.
nit: consider steady_clock
https://en.cppreference.com/w/cpp/chrono/steady_clock
"This clock is not related to wall clock time (for example, it can be time since last reboot), and is most suitable for measuring intervals."
| // Log model creation | ||
| NSLog(@"Creating model ..."); | ||
| auto model = OgaModel::Create(modelPath); | ||
| if (!model) { |
There was a problem hiding this comment.
the C++ API throws exceptions.
might be simplest to just put most of this method into a try {}. propagating the error back to the UI would be a nice touch.
|
|
||
| // Log model creation | ||
| NSLog(@"Creating model ..."); | ||
| auto model = OgaModel::Create(modelPath); |
There was a problem hiding this comment.
performance-wise, it's probably nicer to not re-create the model every time.
There was a problem hiding this comment.
it should definitely be nicer as the first request is usually the slowest (and typically when doing perf testing we do a warmup query first and exclude that from timing data).
| let totalTime = userInfo["totalTime"] as? Int, | ||
| let firstTokenTime = userInfo["firstTokenTime"] as? Int, | ||
| let tokenCount = userInfo["tokenCount"] as? Int { | ||
| stats = "Generated \(tokenCount) tokens in \(totalTime) ms. First token in \(firstTokenTime) ms." |
There was a problem hiding this comment.
Would be nice to have prompt tokens/s as well given that and generation tokens/s are the usual metrics that gets compared. That would require something to return the number of tokens in the prompt (possibly the length of the input sequence post-tokenization) as IIRC it's not 1:1 with the number of words.
|
|
||
| // Log model creation | ||
| NSLog(@"Creating model ..."); | ||
| auto model = OgaModel::Create(modelPath); |
There was a problem hiding this comment.
it should definitely be nicer as the first request is usually the slowest (and typically when doing perf testing we do a warmup query first and exclude that from timing data).
| NSLog(@"Creating tokenizer..."); | ||
| auto tokenizer = OgaTokenizer::Create(*model); | ||
| if (!tokenizer) { | ||
| NSLog(@"Failed to create tokenizer."); | ||
| return; | ||
| } |
There was a problem hiding this comment.
The tokenizer could also be created once and re-used as I believe it's tied to the model not the prompt so can be re-used.
| NSLog(@"First token generated."); | ||
| firstTokenTime = Clock::now(); |
There was a problem hiding this comment.
nit: logging takes time so we want to record firstTokenTime prior to logging.
might also be interesting to include per-token timing in the log as that would help get a picture of performance throughout the generation phase. e.g. is the time per token consistent? if not is the variability random or does it gradually increase/decrease? that can provide hints as to potential causes of performance issues (if there are any).
if we do that we might want to accumulate per-token times in a list and log at the end, as inserting log calls for every token inside the loop could significantly affect overall time taken (esp. is the log call is synchronous which apparently NSLog is).
| break; | ||
| } | ||
|
|
||
| NSLog(@"Decoded token: %s", decode_tokens); |
There was a problem hiding this comment.
nit: might want a setting to control whether we log inside the loop (if we're after the best perf numbers possible we probably don't want to do that), or a way to exclude time in calls to NSLog from the total.
Update Phi-3 iOS application: