-
Notifications
You must be signed in to change notification settings - Fork 1.2k
feature: added _AnalysisConfigGenerator for clarify #3271
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
feature: added _AnalysisConfigGenerator for clarify #3271
Conversation
e90a3ea
to
7f00235
Compare
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.
Two comments:
- Can you please add types?
- Can you please test a config including pre-post-explainability all at once?
Can the generator synthetise the config for all the methods enabled + explainability?
e502748
to
3cefaef
Compare
Done, also created a backlog item to address this for the other/existing methods.
So far there is no API to call for both Bias and Explainability at once. So I cannot. |
weird, but analysis config supports having a job with explainability and bias at once, how is it that we can't do it with the SDK? or am I missing something? |
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.
Changes look fine, one of my question is not addressed, but it should not block your PR. Thank you for your changes, LGTM.
3cefaef
to
1a51f96
Compare
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.
Changes LGTM
1a51f96
to
ea9ea51
Compare
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
fb68b4a
to
1ecdbd4
Compare
1ecdbd4
to
a5cbe0e
Compare
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
51d42e9
to
24bd02e
Compare
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
24bd02e
to
b50a44b
Compare
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
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.
lgtm
PR check for serverless inference failing because of known backend issue, will override and merge this to unblock team. |
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
* feature: extracted analysis config generation for explainability * feature: extracted analysis config generation for bias pre_training * feature: extracted analysis config generation for bias post_training * feature: extracted analysis config generation for bias * feature: simplified job_name creation * feature: extended analysis config generator methods with common logic * feature: refactored _AnalysisConfigGenerator methods * feature: added _last_analysis_config in SageMakerClarifyProcessor * added data types in _AnalysisConfigGenerator methods * applied style formatting to fix build issues Co-authored-by: Yeldos Balgabekov <[email protected]>
Issue #, if available:
Description of changes:
_AnalysisConfigGenerator
class to simplify maintainability of the config creation.job_name
creation part_last_analysis_config
) to store last analysis_config used for a run (to be useful in case of debugging)Testing done:
Unit Tests
Merge Checklist
Put an
x
in the boxes that apply. You can also fill these out after creating the PR. If you're unsure about any of them, don't hesitate to ask. We're here to help! This is simply a reminder of what we are going to look for before merging your pull request.General
Tests
unique_name_from_base
to create resource names in integ tests (if appropriate)By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.