Skip to content
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

jobparser for crd TrainingJob #14

Merged
merged 9 commits into from
Apr 12, 2018
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
*~
vendor/
.glide/
.vscode/
32 changes: 32 additions & 0 deletions pkg/apis/paddlepaddle/v1/trainingjob.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
package v1
Copy link
Collaborator

@wangkuiyi wangkuiyi Apr 3, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The package name here should be

package edl // import "github.com/edl/v1"

instead of

package v1

Please take this file from the Go client library of Google API as a reference.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I noticed that in many of our Go repos, there is a directory pkg. I have never seen such a convention in other Go repos -- not in any well-known libraries like Google's API's Go client, not Go lang's repo itself.

This convention would make it hard for other Go projects to import ours. The authors have to write:

import "github.com/paddlepaddle/edl/pkg/apis/paddlepaddle/v1"

Please be aware that there are two paddlepaddle in the above import path. Ths is pretty redundant.

To make this import path concise and clear, we might want

import "github.com/paddlepaddle/edl/v1"

To achive this, we should have training_job.go in the /v1, instead of /pkg/apis/paddlepaddle/v1.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

pkg directory is widely used in many go repos like https://github.com/coreos/etcd, https://github.com/kubernetes/kubernetes, and the intention is to hide internal implement. I think we can keep the pkg directory and put cmd/edl to edl is more general. Currently since edl does not have any API provided for users to call, we don't have to expose apis directory, /pkg/apis/paddlepaddle/v1 is only for internal use and call the k8s API.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please follow Go's convention and rename this file from trainingjob.go into training_job.go.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done


import (
"encoding/json"
"fmt"
)

// Elastic returns true if the job can scale to more workers.
func (s *TrainingJob) Elastic() bool {
return s.Spec.Trainer.MinInstance < s.Spec.Trainer.MaxInstance
}

// GPU convert Resource Limit Quantity to int
func (s *TrainingJob) GPU() int {
q := s.Spec.Trainer.Resources.Limits.NvidiaGPU()
gpu, ok := q.AsInt64()
if !ok {
// FIXME: treat errors
gpu = 0
}
return int(gpu)
}

// NeedGPU returns true if the job need GPU resource to run.
func (s *TrainingJob) NeedGPU() bool {
return s.GPU() > 0
}

func (s *TrainingJob) String() string {
b, _ := json.MarshalIndent(s, "", " ")
return fmt.Sprintf("%s", b)
}
13 changes: 7 additions & 6 deletions pkg/apis/paddlepaddle/v1/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@ type TrainingJobSpec struct {
Passes int `json:"passes,omitempty"`
Volumes []corev1.Volume `json:"volumes"`
VolumeMounts []corev1.VolumeMount `json:"VolumeMounts"`
//TODO simplify the structure of sub-resource(mengyang)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

// TODO(mengyang): Simplify the structure of sub-resource.

//TrainingJob components.
Master MasterSpec `json:"master"`
Pserver PserverSpec `json:"pserver"`
Expand Down Expand Up @@ -111,12 +112,12 @@ type TrainerJobScaleStatus struct {
type TrainingResourceType string

const (
// MASTER is the master name of TrainingResourceType.
MASTER TrainingResourceType = "MASTER"
// PSERVER is the pserver name of TrainingResourceType.
PSERVER TrainingResourceType = "PSERVER"
// TRAINER is the trainer name of TrainingResourceType.
TRAINER TrainingResourceType = "TRAINER"
// Master is the master name of TrainingResourceType.
Master TrainingResourceType = "MASTER"
// Pserver is the pserver name of TrainingResourceType.
Pserver TrainingResourceType = "PSERVER"
// Trainer is the trainer name of TrainingResourceType.
Trainer TrainingResourceType = "TRAINER"
)

// ResourceState is the state of a type of resource
Expand Down
Loading