fix #1578 and some improvements #2354

squirrelsc · 2020-04-21T09:45:53Z

Add shell support for ssh connection, so that remote script can be started with user environment.

Minor fixes,

Fix gpu_metrics_collector to support pyenv. As pyenv will create one more process, so that original pgrep code always got extra processes, and cannot start gpu_metrics_collector.
Fix NASUI failure on dev-install-node-modules, to create subfolder every time.
Add .gitignore items for dev-install.
Add node --watch for nni_manager for better dev experience.

dev env test passed.

fix gpu_metrics_collector to support pyenv fix NASUI failure on dev-install-node-modules add .gitignore links for dev-install

SparkSnail · 2020-04-24T02:23:51Z

.gitignore

@@ -84,3 +84,8 @@ venv.bak/

 # In case you place source code in ~/nni/
 /experiments
+src/sdk/pynni/nni/nni


why add these folders?

They are links, and created by dev-install.

SparkSnail · 2020-04-24T02:30:01Z

src/nni_manager/training_service/remote_machine/sshClientUtility.ts

        const log: Logger = getLogger();
        log.debug(`remoteExeCommand: command: [${command}]`);
        const deferred: Deferred<RemoteCommandResult> = new Deferred<RemoteCommandResult>();
        let stdout: string = '';
        let stderr: string = '';
        let exitCode: number;
+        let captureStdOut = false;


what is the purpose of captureStdout?

When the shell is using user's interactive environment, there are startup outputs, like what we see when login a SSH shell. This kind of outputs won't be captured, and output.

SparkSnail · 2020-04-24T02:38:59Z

tools/nni_gpu_tool/gpu_metrics_collector.py

        pidList = list(map(int, pidList))
        pidList.remove(os.getpid())
        return not pidList
    else:
-        pgrep_output = subprocess.check_output('pgrep -fxu "$(whoami)" \'python3 -m nni_gpu_tool.gpu_metrics_collector\'', shell=True)
+        pgrep_output = subprocess.check_output(
+            'pgrep -afu "$(whoami)" \'python3 -m nni_gpu_tool.gpu_metrics_collector\'', shell=True)


why use -a instead of -x?
`

-x means exact match and -a means output command text also. It works with default Python, but if user uses multiple Python tools, like pyenv, the command line won't be exact match like this. So use a more complex filter to handle this situation.

ultmaster · 2020-04-24T02:37:19Z

tools/nni_gpu_tool/gpu_metrics_collector.py

        pidList = list(map(int, pidList))
        pidList.remove(os.getpid())
        return not pidList
    else:
-        pgrep_output = subprocess.check_output('pgrep -fxu "$(whoami)" \'python3 -m nni_gpu_tool.gpu_metrics_collector\'', shell=True)
+        pgrep_output = subprocess.check_output(


Our line length limit is 140.

ultmaster · 2020-04-24T02:39:08Z

.gitignore

@@ -84,3 +84,8 @@ venv.bak/

 # In case you place source code in ~/nni/
 /experiments
+src/sdk/pynni/nni/nni


I think these are wrong soft links. We should investigate in why they are created.

chicm-ms · 2020-04-24T02:56:04Z

suggest to merge into a dev branch to run integration-test-remote and integration-test-remote-windows pipelines first

squirrelsc · 2020-04-24T04:16:17Z

suggest to merge into a dev branch to run integration-test-remote and integration-test-remote-windows pipelines first

@chicm-ms
what's process to create a dev branch, and how to enable those tests on dev branch?

leckie-chn · 2020-04-24T06:00:16Z

Makefile

@@ -211,6 +211,7 @@ dev-install-node-modules:
 	sed -ie 's/$(NNI_VERSION_TEMPLATE)/$(NNI_VERSION_VALUE)/' $(NNI_PKG_FOLDER)/package.json
 	ln -sf ${PWD}/src/nni_manager/node_modules $(NNI_PKG_FOLDER)/node_modules
 	ln -sf ${PWD}/src/webui/build $(NNI_PKG_FOLDER)/static
+	mkdir -p $(NASUI_PKG_FOLDER)
 	ln -sf ${PWD}/src/nasui/build $(NASUI_PKG_FOLDER)/build
 	ln -sf ${PWD}/src/nasui/server.js $(NASUI_PKG_FOLDER)/server.js


needs to remove this line, or will give

# Installing NNI Package rm -rf /home/yann/.local/nni ln -sf /home/yann/code/nni/src/nni_manager/dist /home/yann/.local/nni cp src/nni_manager/package.json /home/yann/.local/nni sed -ie 's/999.0.0-developing/v0.2-1270-g06bc964/' /home/yann/.local/nni/package.json ln -sf /home/yann/code/nni/src/nni_manager/node_modules /home/yann/.local/nni/node_modules ln -sf /home/yann/code/nni/src/webui/build /home/yann/.local/nni/static mkdir -p /home/yann/.local/nni/nasui ln -sf /home/yann/code/nni/src/nasui/build /home/yann/.local/nni/nasui/build ln -sf /home/yann/code/nni/src/nasui/server.js /home/yann/.local/nni/nasui/server.js ln: '/home/yann/code/nni/src/nasui/server.js' and '/home/yann/.local/nni/nasui/server.js' are the same file Makefile:207: recipe for target 'dev-install-node-modules' failed make: *** [dev-install-node-modules] Error 1

Didn't hit this issue, and it's not changed this time.

squirrelsc · 2020-04-24T06:45:38Z

Create #2370 as a dev branch. please continue review there.

Chi Song added 4 commits April 21, 2020 17:22

fix Python path bug microsoft#1578

2b34eb3

fix gpu_metrics_collector to support pyenv fix NASUI failure on dev-install-node-modules add .gitignore links for dev-install

update document for dev env improvement.

d9ca0cb

remove log lines

0c15d81

log more error information

383304d

squirrelsc requested review from chicm-ms and QuanluZhang April 21, 2020 09:45

QuanluZhang requested review from SparkSnail, liuzhe-lz and ultmaster April 22, 2020 00:34

Chi Song added 2 commits April 22, 2020 09:20

fix eslint check error

edc7506

fix eslint errors again.

19923d8

SparkSnail reviewed Apr 24, 2020

View reviewed changes

ultmaster mentioned this pull request Apr 24, 2020

Fix softlink for NAS UI in Makefile #2361

Closed

SparkSnail reviewed Apr 24, 2020

View reviewed changes

ultmaster reviewed Apr 24, 2020

View reviewed changes

Chi Song added 2 commits April 24, 2020 13:20

fix auto format rules.

7b86381

fix incorrect created links, and add yarn.lock changes.

ac38785

leckie-chn reviewed Apr 24, 2020

View reviewed changes

improve make file to avoid useless links.

f479251

squirrelsc closed this Apr 24, 2020

squirrelsc deleted the compatible_gpu_metrics branch April 26, 2020 09:41

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix #1578 and some improvements #2354

fix #1578 and some improvements #2354

squirrelsc commented Apr 21, 2020 •

edited

Loading

SparkSnail Apr 24, 2020

squirrelsc Apr 24, 2020

SparkSnail Apr 24, 2020

squirrelsc Apr 24, 2020

SparkSnail Apr 24, 2020

squirrelsc Apr 24, 2020

ultmaster Apr 24, 2020 •

edited

Loading

ultmaster Apr 24, 2020

chicm-ms commented Apr 24, 2020

squirrelsc commented Apr 24, 2020

leckie-chn Apr 24, 2020

squirrelsc Apr 24, 2020

squirrelsc commented Apr 24, 2020

fix #1578 and some improvements #2354

fix #1578 and some improvements #2354

Conversation

squirrelsc commented Apr 21, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ultmaster Apr 24, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

chicm-ms commented Apr 24, 2020

squirrelsc commented Apr 24, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

squirrelsc commented Apr 24, 2020

squirrelsc commented Apr 21, 2020 •

edited

Loading

ultmaster Apr 24, 2020 •

edited

Loading