Support multi trial jobs on same GPU #1109
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
fix remote bug (microsoft#523)
merge master
merge master
merge master
merge master
merge master
merge master
merge master
merge master
merge master
merge master
merge master
merge master
chicm-ms
reviewed
May 27, 2019
QuanluZhang
reviewed
May 27, 2019
| private designatedGpuIndices!: Set<number>; | ||
| private log: Logger; | ||
| private localTrailConfig?: TrialConfig; | ||
| private localConfig?: LocalConfig; | ||
| private isMultiPhase: boolean = false; | ||
| private jobStreamMap: Map<string, ts.Stream>; | ||
| private maxTrialNumPerGPU: number = 1; | ||
| private useActiveGPU: boolean = false; |
There was a problem hiding this comment.
how about init them in constructor?
There was a problem hiding this comment.
fixed, moved to constructor.
QuanluZhang
reviewed
May 27, 2019
| if(trialJobDetail === undefined) { | ||
| throw new Error(`could not get trialJobDetail by id ${trialJobId}`); | ||
| } | ||
| if (trialJobDetail.rmMeta !== undefined && trialJobDetail.rmMeta.occupiedGpuIndexMap !== undefined && trialJobDetail.gpuIndices !== undefined && trialJobDetail.gpuIndices.length > 0) { |
There was a problem hiding this comment.
better to split this line to four lines
There was a problem hiding this comment.
fixed, split to four lines.
QuanluZhang
reviewed
May 27, 2019
| if(rmMeta.occupiedGpuIndexMap !== undefined) { | ||
| let num = rmMeta.occupiedGpuIndexMap.get(gpuInfo.index); | ||
| let maxTrialNumPerGPU: number = rmMeta.maxTrialNumPerGPU? rmMeta.maxTrialNumPerGPU: 1; | ||
| if((num === undefined && (!rmMeta.useActiveGPU && gpuInfo.activeProcessNum === 0 || !rmMeta.useActiveGPU)) || (num !== undefined && num < maxTrialNumPerGPU)) { |
There was a problem hiding this comment.
the same, better to use multiple lines
There was a problem hiding this comment.
suggest 2 lines for this case
There was a problem hiding this comment.
fixed, split to 2 lines.
QuanluZhang
reviewed
May 27, 2019
| } | ||
| rmMeta.occupiedGpuIndexMap.set(gpuInfo.index, num + 1); | ||
| }else { | ||
| rmMeta.occupiedGpuIndexMap = new Map<number, number>(); |
There was a problem hiding this comment.
also reconsider this logic
There was a problem hiding this comment.
fixed, initialize in constructor.
QuanluZhang
reviewed
May 27, 2019
tools/nni_cmd/launcher.py
Outdated
| if request_data['local_config']: | ||
| if request_data['local_config'].get('gpuIndices') and isinstance(request_data['local_config'].get('gpuIndices'), int): | ||
| request_data['local_config']['gpuIndices'] = str(request_data['local_config'].get('gpuIndices')) | ||
| if request_data['local_config'].get('maxTrialNumOnEachGPU'): |
QuanluZhang
reviewed
May 27, 2019
tools/nni_cmd/launcher.py
Outdated
| if request_data['local_config'].get('maxTrialNumOnEachGPU'): | ||
| request_data['local_config']['maxTrialNumOnEachGPU'] = request_data['local_config'].get('maxTrialNumOnEachGPU') | ||
| if request_data['local_config'].get('useActiveGPU'): | ||
| request_data['local_config']['useActiveGPU'] = request_data['local_config'].get('useActiveGPU') |
chicm-ms
reviewed
May 27, 2019
docs/en_US/ExperimentConfig.md
Outdated
|
|
||
| * __useActiveGpu__ | ||
|
|
||
| __useActiveGpu__ is used to specify whether to use a GPU if there is another process. By default, NNI will use the GPU only if there is no another active process in the GPU, if __useActiveGpu__ is set to true, NNI will use the GPU regardless of another processes. |
There was a problem hiding this comment.
need to state that this option is not applicable for NNI on Windows.
added 2 commits
May 27, 2019 17:16
chicm-ms
approved these changes
May 27, 2019
QuanluZhang
approved these changes
May 27, 2019
leelaylay
reviewed
May 27, 2019
| * __useActiveGpu__ | ||
|
|
||
| __useActiveGpu__ is used to specify whether to use a GPU if there is another process. By default, NNI will use the GPU only if there is no another active process in the GPU, if __useActiveGpu__ is set to true, NNI will use the GPU regardless of another processes. This field is not applicable for NNI on Windows. | ||
|
|
There was a problem hiding this comment.
Nice Job!
I test it on my local machine and it works successfully.
It seems that it also works in remote mode when I catch a sight of the code.
I suggest put one example code here.
localConfig:
maxTrialNumPerGpu: 5There was a problem hiding this comment.
good suggestion, I've added an configuration in cifar10-pytorch-example
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No description provided.