Skip to content
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

v1.0 iOS SDK #12

Merged
merged 23 commits into from
May 16, 2024
Merged

v1.0 iOS SDK #12

merged 23 commits into from
May 16, 2024

Conversation

ErisMik
Copy link
Contributor

@ErisMik ErisMik commented May 13, 2024

No description provided.

README.md Outdated Show resolved Hide resolved
binding/ios/PicoLLM.swift Outdated Show resolved Hide resolved
@ErisMik ErisMik requested a review from ksyeo1010 May 14, 2024 21:15
@ksyeo1010 ksyeo1010 requested a review from laves May 14, 2024 21:19
README.md Outdated Show resolved Hide resolved
binding/ios/PicoLLM.swift Outdated Show resolved Hide resolved
binding/ios/PicoLLM.swift Outdated Show resolved Hide resolved
binding/ios/PicoLLM.swift Outdated Show resolved Hide resolved
binding/ios/PicoLLM.swift Outdated Show resolved Hide resolved
demo/ios/PicoLLMChatDemo/PicoLLMChatDemo/ViewModel.swift Outdated Show resolved Hide resolved
demo/ios/README.md Outdated Show resolved Hide resolved
demo/ios/PicoLLMChatDemo/PicoLLMChatDemo/ViewModel.swift Outdated Show resolved Hide resolved
@laves
Copy link
Member

laves commented May 15, 2024

Completion Demo:

  • can we just name the folder 'Completion'? I know we using tag it with some redundant words, but it looks a little clean if there is a 'Chat' and 'Completion' folder in the demo folder
  • If there's an error parsing AccessKey (e.g. didn't replace the default), the error prints to console, but UI just disables the button but doesn't show any message
  • putting ",,," in stop phrases breaks it when you generate. You should split by comma, but then validate each item that you get from the list (just non-null, length > 0). Also, error only showed in console, not the completion area - can you make it show there as well?
  • give more margin on the text area, and more space for the prompt box (from the bottom and sides)
  • remove black border from the text area and curve it if you can
  • move completion settings controls higher so that they're a more comfortable distance from the title
  • if you can move the chevrons even closes together, they'll look more like a single icon
  • a little too much for the icons at the bottom of the text area. It cuts off a bit more text than is necessary.
  • when I reloaded the model, the '# tokens per second' stayed

@laves
Copy link
Member

laves commented May 15, 2024

Chat demo:

  • relevant items from the Completion demo feedback regarding layout/error reporting etc
  • add an extra line after You and picoLLM to give a bit of space to the headings
  • It looks like the picoLLM response is slightly indented but the You prompt is not. I would indent the You prompt
  • when I submit a prompt it should clear the prompt box
  • when I hit the reset/clear button, it shoudl clear the prompt as well
  • make the prompt box hint Message picoLLM (makes it more chatgpt-esque)

binding/ios/PicoLLM.swift Outdated Show resolved Hide resolved
@laves laves merged commit 0e8fc02 into v1.0 May 16, 2024
14 of 15 checks passed
@laves laves deleted the v1.0-ios branch May 16, 2024 17:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants