Model documentation #436
Model documentation #436
Conversation
amandarichardsonn
requested review from
ashao,
mellis13,
al-rigazzi,
billschereriii and
juliaputko
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## smartsim-docs-refresh #436 +/- ##
=========================================================
+ Coverage 84.33% 90.47% +6.14%
=========================================================
Files 58 60 +2
Lines 3223 3738 +515
=========================================================
+ Hits 2718 3382 +664
+ Misses 505 356 -149
|
|
@amandarichardsonn I'd suggest to bring the feature branch up to master, to limit the number of modified files shown in this (and parallel) PRs. |
doc/model.rst
Outdated
|
|
||
| ---------------- | ||
| A Standard Model | ||
| ---------------- |
There was a problem hiding this comment.
total nitpick: could we reduce the total number of lines here by letting lines extend to 80 chars more often?
There was a problem hiding this comment.
That is good practice for sure, Ill go through the doc once Ive finished your review comments
The sphinx-tabs documentation extension uses a white background for the tabs component. This causes readability issues with the theme that we have chosen. A custom CSS has been added to override those components to inherit the overall theme color. [ committed by @mellis13 ] [ reviewed by @al-rigazzi ]
doc/model.rst
Outdated
| ``Experiment`` workflow, such as launching compiled applications, | ||
| running scripts, or performing general computational operations. ``Models`` can be launched with | ||
| other SmartSim entities and :ref:`SmartRedis<dead_link>` to build AI-enabled workflows. | ||
| ``Model`` objects can leverage ML capabilities by utilizing the SmartSim client (:ref:`SmartRedis<dead_link>`) |
There was a problem hiding this comment.
Model objects can leverage ML capabilities by utilizing the SmartSim client (:ref:SmartRedis<dead_link>) to transfer data to a standard Orchestrator to enable other running Models to access the data. Additionally, clients can execute ML models (TF, TF-lite, PyTorch, or ONNX) and TorchScripts stored in the Orchestrator.
->
With the SmartSim client (:ref:SmartRedis<dead_link>), data can be transferred out of the Model into the standalone Orchestrator for use in an ML model (TF, TF-lite, PyTorch, or ONNX), online training process, or additional Model applications.
There was a problem hiding this comment.
you mention ML model, online training process and additional Model apps - what do you mean by online training process? Im wondering if this is repeating ML model
Adds infrastructure to fetch RedisAI's dependencies. This removes the need to call RedisAI's `get_deps.sh` script so that we can fetch newer versions of our machine learning backends than the ones officially supported by RedisAI. Additionally, this upgrades the machine learning python packages required by SmartSim so that they stay up to date with the backends. This in turn allows us to add Python3.10+ONNX support. [ committed by @MattToast ] [ reviewed by @ashao ]
There was a problem hiding this comment.
Thanks for the updates. The examples of key collision prevention I think will really help users make sense of what is a complicated process. Mostly minor comments, but one large theme of using client.set_data_source when performing an operation from within the application that set the data. This applies to all of the sections. I didn't add the same comment to the copy/rename/delete section, but applies there too. After those edits I will do another pass through but we should be close to merging this.
There was a problem hiding this comment.
Mostly minor comments and questions. One general critique for the prefixing section, often the term "script" is used when a more general "application" might be better. I think it's technically correct because all of our examples are scripts, but we don't want users to get in the mindset that all Model are scripts. Otherwise, these are small changes and we are close to merging!
amandarichardsonn
merged commit efcbd74
into
CrayLabs:smartsim-docs-refresh
This PR requests the merge of the Model documentation section into the full craylab SmartSim doc refactor branch.