-
-
Notifications
You must be signed in to change notification settings - Fork 27
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 checkpoint apis #319
base: dev
Are you sure you want to change the base?
add checkpoint apis #319
Conversation
Codecov Report
@@ Coverage Diff @@
## dev #319 +/- ##
==========================================
+ Coverage 30.83% 31.76% +0.92%
==========================================
Files 54 56 +2
Lines 1456 1571 +115
Branches 229 249 +20
==========================================
+ Hits 449 499 +50
- Misses 964 1025 +61
- Partials 43 47 +4
Continue to review full report at Codecov.
|
@@ -125,25 +132,48 @@ class SyftJob internal constructor( | |||
plans: ConcurrentHashMap<String, Plan>, | |||
clientConfig: ClientConfig, | |||
dataLoader: DataLoader, | |||
trainingParameters: TrainingParameters | |||
trainingParameters: TrainingParameters, | |||
checkPointSerializer: CheckPointSerializer? = null |
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 pass this in the constructor?
Also, why clientConfig
had to be passed here and not in the constructor? Can a job execute different plans with different client configurations?
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.
because clientConfig
will only be available when the job is on ReadyStatus
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.
Go!
Description
Please include a summary of the change, the motivation, and any additional context that will help others understand your PR. If it closes one or more open issues, please tag them as described here.
Affected Dependencies
List any dependencies that are required for this change.
How has this been tested?
Checklist