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

fix no log from subprocess on trial #3653

Merged
merged 10 commits into from
May 27, 2021
+6 −3
Merged

fix no log from subprocess on trial #3653

Changes from 1 commit
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
ee40d6c
fix no log from subprocess on trial
May 19, 2021
e0eaef9
fix no log view from web UI
May 25, 2021
437aaac
fix ut
May 26, 2021
750e468
fix ut
May 26, 2021
dba74d5
fix ut
May 26, 2021
16f327b
fix ut
May 26, 2021
d72aa02
fix ut
May 26, 2021
c2beb9f
fix -encoding not working on win32
May 26, 2021
8a5115a
add new type(TRIAL_STDOUT) for log
May 27, 2021
a00aa31
revert ut
May 27, 2021
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
4 changes: 2 additions & 2 deletions ts/nni_manager/training_service/local/localTrainingService.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -414,13 +414,13 @@ class LocalTrainingService implements TrainingService {
if (process.platform === 'win32') {
script.push(`cd $env:NNI_CODE_DIR`);
script.push(
`cmd.exe /c ${this.config.trialCommand} 2>&1 | Out-File "${path.join(workingDirectory, 'stderr')}" -encoding utf8`,
`cmd.exe /c ${this.config.trialCommand} 1> "${path.join(workingDirectory, 'stdout')}" 2> "${path.join(workingDirectory, 'stderr')}" -encoding utf8`,
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not familiar with cmd. Can someone check this?

`$NOW_DATE = [int64](([datetime]::UtcNow)-(get-date "1/1/1970")).TotalSeconds`,
`$NOW_DATE = "$NOW_DATE" + (Get-Date -Format fff).ToString()`,
`Write $LASTEXITCODE " " $NOW_DATE | Out-File "${path.join(workingDirectory, '.nni', 'state')}" -NoNewline -encoding utf8`);
} else {
script.push(`cd $NNI_CODE_DIR`);
script.push(`eval ${this.config.trialCommand} 2>"${path.join(workingDirectory, 'stderr')}"`);
script.push(`eval ${this.config.trialCommand} 1>"${path.join(workingDirectory, 'stdout')} 2>"${path.join(workingDirectory, 'stderr')}"`);
Copy link
Contributor

Choose a reason for hiding this comment

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

We still cannot view stdout from WebUI. That should be a problem?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

FIxed,

After some investigate about this issue, here are some result:
option1:
sys.stdout redirect for trial.py (->trial.log)
result:
only trial.py log show up, no subprocess log. (This is the issue linked on this PR)

option2:
sys.stdout redirect for trial.py (->trial.log)
launch trial.py with command "1 > stdout"
result:
trial.py log is in trial.log, subprocess log is in stdout.

option3:
sys.stdout redirect for trial.py (->trial.log)
launch trial.py with command "1 > trial.log"
result:
Thread save exceptions.

option4:
launch trial.py with command "1 > stdout"
result:
trial.py log and subprocess log are in stdout.

I recommend remove sys.stdout redirect which is only works for local platform.

(Somehow, It is no works for subprocess output if only setup "sys.stdout" on parent process, You have to change the code by "subprocess.Popen(your_command, stdout=sys.stdout)")

if (process.platform === 'darwin') {
// https://superuser.com/questions/599072/how-to-get-bash-execution-time-in-milliseconds-under-mac-os-x
// Considering the worst case, write 999 to avoid negative duration
Expand Down