search

sambanova / ai-starter-kit

Other
81 stars 29 forks source link

Plotly and benchmarking #326

Closed snova-michaelw closed 1 month ago

snova-michaelw commented 1 month ago

Two main flavours of edits to this PR, all on the Benchmarking AISK. All have been tested by building a VENV from scratch and running the analyze-results notebook at the Streamlit app with no issues. So:

  1. Cleaned up all references to sncloud and sambastudio to instead read SambaNova Cloud and SambaStudio, respectively. It just makes the UI a little bit cleaner and more merketing-focussed.
  2. Replaced the non-interactive, slightly-ugly Seaborn charts with fully-interactive, SN-themed, dark-mode plotly charts. They look nicer, and they're more useful: a. Boxplots are now violin plots, which are much more informative for the non-Normal data that the benchmarks produce b. Improved labels and descriptions are a bit clearer c. Added the Gantt chart to the Streamlit UI (it had been buried in a notebook before)

Also did a number of typo fixes, cleaned up some of the documentation, and improved the benchmarking harness so that it no longer rounds certain timestamps to the nearest second (which was introducing some large errors, like in the Gantt chart). newplot (1) newplot (2)

snova-michaelw commented 1 month ago

Thanks for this PR @snova-michaelw . I've dropped some comments regarding your changes.

OK, I've now resolved as much as I can. The one pending issue is the most elegant way to report metrics for the Cloud, where batch size is dynamic, mixed, and opaque to the end user. That issue is beyond the scope of this PR, I reckon.

I've also merged/resolved new commits to main in the meantime, and I've checked that the code still runs. I think it should be good for auto-merging, then.

snova-rodrigom commented 1 month ago

@snova-amitk if there are no other questions from your end, I think we can merge this changes. Thanks!

snova-amitk commented 1 month ago

Thanks @snova-michaelw! This is a great update to the kit.