Skip to content
This repository has been archived by the owner on Sep 18, 2024. It is now read-only.

rename preCommand to pythonPath #3410

Merged
merged 8 commits into from
Mar 5, 2021
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion docs/en_US/TrainingService/RemoteMachineMode.rst
Original file line number Diff line number Diff line change
Expand Up @@ -144,7 +144,7 @@ Files in ``codeDir`` will be uploaded to remote machines automatically. You can
Configure python environment
^^^^^^^^^^^^^^^^^^^^^^^^^^^^

By default, commands and scripts will be executed in the default environment in remote machine. If there are multiple python virtual environments in your remote machine, and you want to run experiments in a specific environment, then use **preCommand** to specify a python environment on your remote machine.
By default, commands and scripts will be executed in the default environment in remote machine. If there are multiple python virtual environments in your remote machine, and you want to run experiments in a specific environment, then use **pythonPath** to specify a python path environment on your remote machine.
Copy link
Contributor

Choose a reason for hiding this comment

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

python path evnironment seems strange, python environment is enough.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.


Use ``examples/trials/mnist-tfv2`` as the example. Below is content of ``examples/trials/mnist-tfv2/config_remote.yml``\ :

Expand Down
2 changes: 1 addition & 1 deletion docs/en_US/reference/experiment_config.rst
Original file line number Diff line number Diff line change
Expand Up @@ -538,7 +538,7 @@ If `trialGpuNumber`_ is less than the length of this value, only a subset will b
This will be used as ``CUDA_VISIBLE_DEVICES`` environment variable.


trialPrepareCommand
trialPythonPath
Copy link
Contributor

Choose a reason for hiding this comment

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

what is the difference between pythonPath and trialPythonPath?

Copy link
Contributor

Choose a reason for hiding this comment

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

pythonPath is config v1
trialPythonPath is config v2

Copy link
Contributor

Choose a reason for hiding this comment

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

why use different names?

Copy link
Contributor

Choose a reason for hiding this comment

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

Just follow the previous naming way: preCommand is config v1, trialPrepareCommand is config v2. Or use pythonPath is enough? @liuzhe-lz

Copy link
Contributor

@liuzhe-lz liuzhe-lz Mar 3, 2021

Choose a reason for hiding this comment

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

I think pythonPath is enough.
It was named trialPrepareCommand because the command executes once per trial. preCommand made me guess it would execute once per machine.

Copy link
Contributor

Choose a reason for hiding this comment

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

could someone explain when to use pythonPath and when trialPythonPath should be used?

-------------------

Command(s) to run before launching each trial.
Copy link
Contributor

Choose a reason for hiding this comment

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

The path will insert to PATH before launching each trial.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

Expand Down
2 changes: 1 addition & 1 deletion docs/tools/md2rst.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ def single_line_process(line):
line = re.sub(r'\\\* \*\*(.*?)\*\* \(\\\*\s*(.*?)\s*\*\\ \)', r'* \1 (\2)', line)
line = re.sub(r'\<(.*)\.md(\>|#)', r'<\1.rst\2', line)
line = re.sub(r'`\*\*(.*?)\*\* <#(.*?)>`__', r'`\1 <#\2>`__', line)
line = re.sub(r'\*\* (classArgs|stop|FLOPS.*?|pruned.*?|large.*?|path|preCommand|2D.*?|codeDirectory|ps|worker|Tuner|Assessor)\*\*',
line = re.sub(r'\*\* (classArgs|stop|FLOPS.*?|pruned.*?|large.*?|path|pythonPath|2D.*?|codeDirectory|ps|worker|Tuner|Assessor)\*\*',
r' **\1**', line)

line = line.replace('.. code-block:::: bash', '.. code-block:: bash')
Expand Down
6 changes: 1 addition & 5 deletions examples/trials/mnist-tfv2/config_remote.yml
Original file line number Diff line number Diff line change
Expand Up @@ -24,9 +24,5 @@ machineList:
- ip: ${replace_to_your_remote_machine_ip}
username: ${replace_to_your_remote_machine_username}
sshKeyPath: ${replace_to_your_remote_machine_sshKeyPath}
# Pre-command will be executed before the remote machine executes other commands.
# Below is an example of specifying python environment.
# If you want to execute multiple commands, please use "&&" to connect them.
# preCommand: source ${replace_to_absolute_path_recommended_here}/bin/activate
# preCommand: source ${replace_to_conda_path}/bin/activate ${replace_to_conda_env_name}
preCommand: export PATH=${replace_to_python_environment_path_in_your_remote_machine}:$PATH
pythonPath: ${replace_to_python_environment_path_in_your_remote_machine}
Copy link
Contributor

Choose a reason for hiding this comment

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

better to add comments to show concrete path examples here

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

Copy link
Contributor

Choose a reason for hiding this comment

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

The doc said "The path will insert to PATH before launching each trial". Is /usr/bin/python3.7 supposed to be a directory?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It is a directory, "python3.7" is a folder name..I will rename it to "yourPythonPath" to avoid the misunderstanding.

Copy link
Contributor

@liuzhe-lz liuzhe-lz Mar 3, 2021

Choose a reason for hiding this comment

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

Then I suggest /opt/python3.7/bin for Linux example.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

2 changes: 1 addition & 1 deletion nni/experiment/config/convert.py
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,7 @@ def _handle_training_service(ts, data):
'gpuIndices': _convert_gpu_indices(machine.get('gpuIndices')),
'maxTrialNumPerGpu': machine.get('maxTrialNumPerGpu'),
'useActiveGpu': machine.get('useActiveGpu'),
'preCommand': machine.get('trialPrepareCommand')
'pythonPath': machine.get('trialPythonPath')
}
machine_v1 = {k: v for k, v in machine_v1.items() if v is not None}
data['machineList'].append(machine_v1)
Expand Down
2 changes: 1 addition & 1 deletion nni/experiment/config/remote.py
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ class RemoteMachineConfig(ConfigBase):
use_active_gpu: bool = False
max_trial_number_per_gpu: int = 1
gpu_indices: Optional[Union[List[int], str]] = None
trial_prepare_command: Optional[str] = None
trial_python_path: Optional[str] = None

_canonical_rules = {
'ssh_key_file': util.canonical_path,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ describe('ShellExecutor test', () => {
await executor.close();
});

it('Test preCommand-1', async () => {
it('Test pythonPath-1', async () => {
if (skip) {
return;
}
Expand All @@ -99,16 +99,16 @@ describe('ShellExecutor test', () => {
await executor.close();
});

it('Test preCommand-2', async () => {
it('Test pythonPath-2', async () => {
if (skip) {
return;
}
const executor: ShellExecutor = new ShellExecutor();
rmMeta.preCommand = isWindows ? "set TEST_PRE_COMMAND=test_pre_command" : "export TEST_PRE_COMMAND=test_pre_command";
rmMeta.pythonPath = "test_python_path";
await executor.initialize(rmMeta);
const command = isWindows ? "python -c \"import os; print(os.environ.get(\'TEST_PRE_COMMAND\'))\"" : "python3 -c \"import os; print(os.environ.get(\'TEST_PRE_COMMAND\'))\"";
const command = isWindows ? "python -c \"import os; print(os.environ.get(\'PATH\'))\"" : "python3 -c \"import os; print(os.environ.get(\'PATH\'))\"";
const result = (await executor.executeScript(command, false, false)).stdout.replace(/[\ +\r\n]/g, "");
chai.expect(result).eq("test_pre_command");
chai.expect(result).contain("test_python_path");
await executor.close();
});
});