Support 3rd-party training service #3662
Conversation
ts/nni_manager/common/nniConfig.ts
Outdated
| } else { | ||
| configPath = path.join(os.homedir(), '.config/nni', fileName); | ||
| } | ||
| return fs.readFileSync(configPath).toString(); |
There was a problem hiding this comment.
This is not consistent with the logic in nni.runtime.config. What will happen in the case of conda?
|
|
||
| export async function getCustomEnvironmentServiceConfig(name: string): Promise<CustomEnvironmentServiceConfig | null> { | ||
| const configJson = await readConfigFile('training_services.json'); | ||
| const config = JSON.parse(configJson); |
There was a problem hiding this comment.
try catch format error?
There was a problem hiding this comment.
The file is programmatically generated by NNI and this function is only invoked when user asks a custom training service.
So I prefer to just crash if the json file is broken.
| @@ -32,9 +31,9 @@ export class OpenPaiEnvironmentService extends EnvironmentService { | |||
| private experimentId: string; | |||
| private config: FlattenOpenpaiConfig; | |||
|
|
|||
| constructor(config: ExperimentConfig) { | |||
| constructor(_experimentRootDir: string, experimentId: string, config: ExperimentConfig) { | |||
There was a problem hiding this comment.
Why pass rootDir and expId from constructor function?
There was a problem hiding this comment.
Because they are stored in "module scope objects" and custom training services cannot access them.
We must either (1) provide stateful API function to get experiment ID, (2) pass experiment ID through parameter.
According to the meeting last week, (2) is preferred by most people.
| print_error('Bad experiment config class') | ||
| return | ||
|
|
||
| try: |
There was a problem hiding this comment.
What if user register an existed training service name? such as 'local' or 'aml'?
There was a problem hiding this comment.
Oh that's a problem, I'll push a fix.
Doc and example (template) will be added later.