-
Notifications
You must be signed in to change notification settings - Fork 507
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
Add model access control documentation for ML Commons #4223
Conversation
Signed-off-by: Fanit Kolchina <[email protected]>
Signed-off-by: Fanit Kolchina <[email protected]>
Signed-off-by: Fanit Kolchina <[email protected]>
Signed-off-by: Fanit Kolchina <[email protected]>
Signed-off-by: Fanit Kolchina <[email protected]>
|
||
- `public`: All users who have access to the cluster can access this model group. | ||
- `private`: Only the model owner or an admin user can access this model group. | ||
- `restricted`: The owner, an admin user, or any user who shares one of the model group's backend roles can access any model in this model group. When creating a `restricted` model group, the owner must attach one or more of the owner's backend roles to the model. For a user to be able to access a model group, one of the user's backend roles must match one of the model group's backend roles. This grants the user permissions associated with the user role to which that backend role is mapped. For example, if the backend role is mapped to the `ml_full_access` user role, users with this role can use the Register, Deploy, Predict, Delete, Search and Get Model APIs on any model in this model group. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ml_full_access is the permission/security role actually. Also its not necessary a backend role should be mapped to these permission roles, a user can directly be mapped to these permission roles. For example, a user is direclt mapped to "ml_full_access" and has a backend role of that of a model group, then then can perform all the said actions on the models of this group. We might have to rephrase this paragraph a little.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for the clarification, reworded.
Signed-off-by: Fanit Kolchina <[email protected]>
_ml-commons-plugin/api.md
Outdated
} | ||
``` | ||
|
||
## Registering a model | ||
|
||
Use the register operation to register a custom model to a model index. ML Commons splits the model into smaller chunks and saves those chunks in the model's index. | ||
|
||
### Model access control considerations | ||
|
||
For clusters with model access control enabled, the following users can register new model versions for model groups with the specified access levels: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How about add a link to the cluster setting for model access control enabled
? So user can know how to enable model access control
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ignore the comment, see line 171 already has a link to model access control page.
_ml-commons-plugin/api.md
Outdated
"description": "test model", | ||
"model_format": "TORCH_SCRIPT", | ||
"model_group_id": "FTNlQ4gBYW0Qyy5ZoxfR", | ||
"model_content_hash_value": "9376c2ebd7c83f99ec2526323786c348d2382e6d86576f750c89ea544d6bbb14", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
model_content_hash_value
is mandatory, add this field to filed table above? Explain that the hash value is using sha-256
_ml-commons-plugin/api.md
Outdated
"framework_type": "sentence_transformers", | ||
"all_config": "{\"_name_or_path\":\"nreimers/MiniLM-L6-H384-uncased\",\"architectures\":[\"BertModel\"],\"attention_probs_dropout_prob\":0.1,\"gradient_checkpointing\":false,\"hidden_act\":\"gelu\",\"hidden_dropout_prob\":0.1,\"hidden_size\":384,\"initializer_range\":0.02,\"intermediate_size\":1536,\"layer_norm_eps\":1e-12,\"max_position_embeddings\":512,\"model_type\":\"bert\",\"num_attention_heads\":12,\"num_hidden_layers\":6,\"pad_token_id\":0,\"position_embedding_type\":\"absolute\",\"transformers_version\":\"4.8.2\",\"type_vocab_size\":2,\"use_cache\":true,\"vocab_size\":30522}" | ||
}, | ||
"url": "https://github.com/opensearch-project/ml-commons/raw/2.x/ml-algorithms/src/test/resources/org/opensearch/ml/engine/algorithms/text_embedding/all-MiniLM-L6-v2_torchscript_sentence-transformer.zip?raw=true" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This file is in github test code, we add this as the pre-trained model not uploaded to model repo yet.
How about change to https://artifacts.opensearch.org/models/ml-models/huggingface/sentence-transformers/all-MiniLM-L6-v2/1.0.1/torch_script/sentence-transformers_all-MiniLM-L6-v2-1.0.1-torch_script.zip
And the model_content_hash_value
change to c15f0d2e62d872be5b5bc6c84d2e0f4921541e29fefbef51d59cc10a8ae30e0f
_ml-commons-plugin/api.md
Outdated
} | ||
``` | ||
|
||
## Deploying a model | ||
|
||
The deploy model operation reads the model's chunks from the model index and then creates an instance of the model to cache into memory. This operation requires the `model_id`. | ||
|
||
### Model access control considerations | ||
|
||
For clusters with model access control enabled, the following users can deploy models in model groups with the specified access levels: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This part seems same with register model API. Actually model access control will be applied to all APIs for uploaded model. Maybe we don't need to specifically mention the access control for register and deploy model API.
"created_time" : 1665961344044, | ||
"last_uploaded_time" : 1665961373000, | ||
"last_loaded_time" : 1665961815959, | ||
"total_chunks" : 9 | ||
} | ||
``` | ||
|
||
## Registering a model |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add register model group API? User need to register model group first, then register model version to model group.
Signed-off-by: Fanit Kolchina <[email protected]>
Signed-off-by: Fanit Kolchina <[email protected]>
Signed-off-by: Fanit Kolchina <[email protected]>
Signed-off-by: Fanit Kolchina <[email protected]>
"last_updated_time": 1684362571300, | ||
"name": "model_group_test", | ||
"description": "This is an example description", | ||
"tags": { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we remove the tags fields and the keys from this response? We actually removed this field and this happened to be an old response.
|
||
- `public`: All users who have access to the cluster can access this model group. | ||
- `private`: Only the model owner or an admin user can access this model group. | ||
- `restricted`: The owner, an admin user, or any user who shares one of the model group's backend roles can access any model in this model group. When creating a `restricted` model group, the owner must attach one or more of the owner's backend roles to the model. For a user to be able to access a model group, one of the user's backend roles must match one of the model group's backend roles. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe remove the last line here. It seems the same as first sentence.
Signed-off-by: Fanit Kolchina <[email protected]>
Signed-off-by: Fanit Kolchina <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let me know if you have questions for any of the comments/suggestions. We can revisit any of these a second time.
_ml-commons-plugin/api.md
Outdated
|
||
- `public` model group: Any user. | ||
- `restricted` model group: Only the model owner or users with at least one backend role matching one of the backend roles of this model group. | ||
- `private` model group: Only the model owner. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there an extra space in front of private
?
Only admin users can assign backend roles to users. | ||
{: .note} | ||
|
||
For example, consider two users: `alice` and `bob`. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would set up this example with different language. Something like, "When assigning backend roles, consider the following example:"
And I think the language from the Security Analytics Security page would fit better here, too (same example but worded better).
https://opensearch.org/docs/latest/security-analytics/security/
Signed-off-by: Fanit Kolchina <[email protected]>
Co-authored-by: Chris Moore <[email protected]> Signed-off-by: kolchfa-aws <[email protected]>
Signed-off-by: Fanit Kolchina <[email protected]>
@cwillum Reworded and implemented comments. Please review again when you get a chance. Thanks! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please see the edits and suggested comments. Overall content is in good shape. Edits are copyedits for style guide compliance and clarity of information. Please let me know if we need to sync to discuss any editorial feedback. Thanks!
_ml-commons-plugin/api.md
Outdated
|
||
In order to train tasks through the API, three inputs are required. | ||
In order to train tasks through the API, three inputs are required: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In order to train tasks through the API, three inputs are required: | |
To train tasks through the API, three inputs are required: |
_ml-commons-plugin/api.md
Outdated
|
||
- Algorithm name: Must be one of a [FunctionName](https://github.com/opensearch-project/ml-commons/blob/1.3/common/src/main/java/org/opensearch/ml/common/parameter/FunctionName.java). This determines what algorithm the ML Engine runs. To add a new function, see [How To Add a New Function](https://github.com/opensearch-project/ml-commons/blob/main/docs/how-to-add-new-function.md). | ||
- Model hyper parameters: Adjust these parameters to make the model train better. | ||
- Model hyperparameters: Adjust these parameters to make the model train better. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Suggested rewrite: "Model hyperparameters: Adjust these parameters to improve model accuracy."
_ml-commons-plugin/api.md
Outdated
|
||
- Algorithm name: Must be one of a [FunctionName](https://github.com/opensearch-project/ml-commons/blob/1.3/common/src/main/java/org/opensearch/ml/common/parameter/FunctionName.java). This determines what algorithm the ML Engine runs. To add a new function, see [How To Add a New Function](https://github.com/opensearch-project/ml-commons/blob/main/docs/how-to-add-new-function.md). | ||
- Model hyper parameters: Adjust these parameters to make the model train better. | ||
- Model hyperparameters: Adjust these parameters to make the model train better. | ||
- Input data: The data input that trains the ML model, or applies the ML models to predictions. You can input data in two ways, query against your index or use data frame. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- Input data: The data input that trains the ML model, or applies the ML models to predictions. You can input data in two ways, query against your index or use data frame. | |
- Input data: The data that trains the ML model, or applies the ML models to predictions. You can input data in two ways, query against your index or use a data frame. |
``` | ||
{% include copy-curl.html %} | ||
|
||
The response contains the model ID of the model version: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The response contains the model ID of the model version: | |
The response contains the model ID of the model version: |
The response contains the model ID of the model version: | |
The response contains the `model_ID` of the model version: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Similarly to the above comment, I think it's ok.
_ml-commons-plugin/api.md
Outdated
model_id | string | Returns runtime data for a specific model. You can string together multiple `model_id`s to return multiple model profiles. | ||
tasks | string | Returns runtime data for a specific task. You can string together multiple `task_id`s to return multiple task profiles. | ||
model_id | String | Returns runtime data for a specific model. You can string together multiple `model_id`s to return multiple model profiles. | ||
tasks | String | Returns runtime data for a specific task. You can string together multiple `task_id`s to return multiple task profiles. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should "tasks" be task_id
? Or, model_id
be "models?" See lines 514 and 515.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
tasks
is correct. I put everything in tic marks though.
_ml-commons-plugin/index.md
Outdated
@@ -13,17 +13,16 @@ ML Commons for OpenSearch eases the development of machine learning features by | |||
|
|||
Interaction with the ML Commons plugin occurs through either the [REST API]({{site.url}}{{site.baseurl}}/ml-commons-plugin/api) or [`ad`]({{site.url}}{{site.baseurl}}/search-plugins/sql/ppl/functions#ad) and [`kmeans`]({{site.url}}{{site.baseurl}}/search-plugins/sql/ppl/functions#kmeans) Piped Processing Language (PPL) commands. | |||
|
|||
Models [trained]({{site.url}}{{site.baseurl}}/ml-commons-plugin/api#train-model) through the ML Commons plugin support model-based algorithms such as kmeans. After you've trained a model enough so that it meets your precision requirements, you can apply the model to [predict]({{site.url}}{{site.baseurl}}/ml-commons-plugin/api#predict) new data safely. | |||
Models [trained]({{site.url}}{{site.baseurl}}/ml-commons-plugin/api#train-model) through the ML Commons plugin support model-based algorithms such as k-means. After you've trained a model enough so that it meets your precision requirements, you can apply the model to [predict]({{site.url}}{{site.baseurl}}/ml-commons-plugin/api#predict) new data safely. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Line 12: Define ML on first mention as follows: "ML Commons for...of machine learning (ML)..."
Signed-off-by: Fanit Kolchina <[email protected]>
Co-authored-by: Melissa Vagi <[email protected]> Signed-off-by: kolchfa-aws <[email protected]>
Signed-off-by: Fanit Kolchina <[email protected]>
Signed-off-by: Fanit Kolchina <[email protected]>
Signed-off-by: Fanit Kolchina <[email protected]>
Signed-off-by: Fanit Kolchina <[email protected]>
Signed-off-by: Fanit Kolchina <[email protected]>
Signed-off-by: Fanit Kolchina <[email protected]>
* Add model access control documentation for ML Commons Signed-off-by: Fanit Kolchina <[email protected]> * Remove permissions for delete API Signed-off-by: Fanit Kolchina <[email protected]> * Add copy buttons Signed-off-by: Fanit Kolchina <[email protected]> * Updated model-level APIs Signed-off-by: Fanit Kolchina <[email protected]> * Add delete model Signed-off-by: Fanit Kolchina <[email protected]> * Reworded role-related text Signed-off-by: Fanit Kolchina <[email protected]> * Implemented tech review comments Signed-off-by: Fanit Kolchina <[email protected]> * Rewording Signed-off-by: Fanit Kolchina <[email protected]> * Remove experimental warning Signed-off-by: Fanit Kolchina <[email protected]> * Register a model group in note format Signed-off-by: Fanit Kolchina <[email protected]> * Implement tech review comments Signed-off-by: Fanit Kolchina <[email protected]> * Resolved Vale comments Signed-off-by: Fanit Kolchina <[email protected]> * Remove space Signed-off-by: Fanit Kolchina <[email protected]> * Apply suggestions from code review Co-authored-by: Chris Moore <[email protected]> Signed-off-by: kolchfa-aws <[email protected]> * Implemented doc review feedback Signed-off-by: Fanit Kolchina <[email protected]> * Implemented editorial comments Signed-off-by: Fanit Kolchina <[email protected]> * Apply suggestions from code review Co-authored-by: Melissa Vagi <[email protected]> Signed-off-by: kolchfa-aws <[email protected]> * Add more editorial comments Signed-off-by: Fanit Kolchina <[email protected]> * Add more editorial comments Signed-off-by: Fanit Kolchina <[email protected]> * Fix links Signed-off-by: Fanit Kolchina <[email protected]> * Fix more links Signed-off-by: Fanit Kolchina <[email protected]> --------- Signed-off-by: Fanit Kolchina <[email protected]> Signed-off-by: kolchfa-aws <[email protected]> Co-authored-by: Chris Moore <[email protected]> Co-authored-by: Melissa Vagi <[email protected]>
* Add model access control documentation for ML Commons Signed-off-by: Fanit Kolchina <[email protected]> * Remove permissions for delete API Signed-off-by: Fanit Kolchina <[email protected]> * Add copy buttons Signed-off-by: Fanit Kolchina <[email protected]> * Updated model-level APIs Signed-off-by: Fanit Kolchina <[email protected]> * Add delete model Signed-off-by: Fanit Kolchina <[email protected]> * Reworded role-related text Signed-off-by: Fanit Kolchina <[email protected]> * Implemented tech review comments Signed-off-by: Fanit Kolchina <[email protected]> * Rewording Signed-off-by: Fanit Kolchina <[email protected]> * Remove experimental warning Signed-off-by: Fanit Kolchina <[email protected]> * Register a model group in note format Signed-off-by: Fanit Kolchina <[email protected]> * Implement tech review comments Signed-off-by: Fanit Kolchina <[email protected]> * Resolved Vale comments Signed-off-by: Fanit Kolchina <[email protected]> * Remove space Signed-off-by: Fanit Kolchina <[email protected]> * Apply suggestions from code review Co-authored-by: Chris Moore <[email protected]> Signed-off-by: kolchfa-aws <[email protected]> * Implemented doc review feedback Signed-off-by: Fanit Kolchina <[email protected]> * Implemented editorial comments Signed-off-by: Fanit Kolchina <[email protected]> * Apply suggestions from code review Co-authored-by: Melissa Vagi <[email protected]> Signed-off-by: kolchfa-aws <[email protected]> * Add more editorial comments Signed-off-by: Fanit Kolchina <[email protected]> * Add more editorial comments Signed-off-by: Fanit Kolchina <[email protected]> * Fix links Signed-off-by: Fanit Kolchina <[email protected]> * Fix more links Signed-off-by: Fanit Kolchina <[email protected]> --------- Signed-off-by: Fanit Kolchina <[email protected]> Signed-off-by: kolchfa-aws <[email protected]> Co-authored-by: Chris Moore <[email protected]> Co-authored-by: Melissa Vagi <[email protected]> Signed-off-by: Melissa Vagi <[email protected]>
This reverts commit e69009b. Signed-off-by: Melissa Vagi <[email protected]>
…ject#4223) * Add model access control documentation for ML Commons Signed-off-by: Fanit Kolchina <[email protected]> * Remove permissions for delete API Signed-off-by: Fanit Kolchina <[email protected]> * Add copy buttons Signed-off-by: Fanit Kolchina <[email protected]> * Updated model-level APIs Signed-off-by: Fanit Kolchina <[email protected]> * Add delete model Signed-off-by: Fanit Kolchina <[email protected]> * Reworded role-related text Signed-off-by: Fanit Kolchina <[email protected]> * Implemented tech review comments Signed-off-by: Fanit Kolchina <[email protected]> * Rewording Signed-off-by: Fanit Kolchina <[email protected]> * Remove experimental warning Signed-off-by: Fanit Kolchina <[email protected]> * Register a model group in note format Signed-off-by: Fanit Kolchina <[email protected]> * Implement tech review comments Signed-off-by: Fanit Kolchina <[email protected]> * Resolved Vale comments Signed-off-by: Fanit Kolchina <[email protected]> * Remove space Signed-off-by: Fanit Kolchina <[email protected]> * Apply suggestions from code review Co-authored-by: Chris Moore <[email protected]> Signed-off-by: kolchfa-aws <[email protected]> * Implemented doc review feedback Signed-off-by: Fanit Kolchina <[email protected]> * Implemented editorial comments Signed-off-by: Fanit Kolchina <[email protected]> * Apply suggestions from code review Co-authored-by: Melissa Vagi <[email protected]> Signed-off-by: kolchfa-aws <[email protected]> * Add more editorial comments Signed-off-by: Fanit Kolchina <[email protected]> * Add more editorial comments Signed-off-by: Fanit Kolchina <[email protected]> * Fix links Signed-off-by: Fanit Kolchina <[email protected]> * Fix more links Signed-off-by: Fanit Kolchina <[email protected]> --------- Signed-off-by: Fanit Kolchina <[email protected]> Signed-off-by: kolchfa-aws <[email protected]> Co-authored-by: Chris Moore <[email protected]> Co-authored-by: Melissa Vagi <[email protected]>
* Add model access control documentation for ML Commons Signed-off-by: Fanit Kolchina <[email protected]> * Remove permissions for delete API Signed-off-by: Fanit Kolchina <[email protected]> * Add copy buttons Signed-off-by: Fanit Kolchina <[email protected]> * Updated model-level APIs Signed-off-by: Fanit Kolchina <[email protected]> * Add delete model Signed-off-by: Fanit Kolchina <[email protected]> * Reworded role-related text Signed-off-by: Fanit Kolchina <[email protected]> * Implemented tech review comments Signed-off-by: Fanit Kolchina <[email protected]> * Rewording Signed-off-by: Fanit Kolchina <[email protected]> * Remove experimental warning Signed-off-by: Fanit Kolchina <[email protected]> * Register a model group in note format Signed-off-by: Fanit Kolchina <[email protected]> * Implement tech review comments Signed-off-by: Fanit Kolchina <[email protected]> * Resolved Vale comments Signed-off-by: Fanit Kolchina <[email protected]> * Remove space Signed-off-by: Fanit Kolchina <[email protected]> * Apply suggestions from code review Co-authored-by: Chris Moore <[email protected]> Signed-off-by: kolchfa-aws <[email protected]> * Implemented doc review feedback Signed-off-by: Fanit Kolchina <[email protected]> * Implemented editorial comments Signed-off-by: Fanit Kolchina <[email protected]> * Apply suggestions from code review Co-authored-by: Melissa Vagi <[email protected]> Signed-off-by: kolchfa-aws <[email protected]> * Add more editorial comments Signed-off-by: Fanit Kolchina <[email protected]> * Add more editorial comments Signed-off-by: Fanit Kolchina <[email protected]> * Fix links Signed-off-by: Fanit Kolchina <[email protected]> * Fix more links Signed-off-by: Fanit Kolchina <[email protected]> --------- Signed-off-by: Fanit Kolchina <[email protected]> Signed-off-by: kolchfa-aws <[email protected]> Co-authored-by: Chris Moore <[email protected]> Co-authored-by: Melissa Vagi <[email protected]>
Fixes #3514
Checklist
For more information on following Developer Certificate of Origin and signing off your commits, please check here.