-
Notifications
You must be signed in to change notification settings - Fork 1.8k
refactor experiment summary file(to fix componentWillReceiveProps waring) and click other areas to close panel #2734
Changes from 6 commits
c95bf9e
ac60633
63bfd1c
7094329
5525320
ed098a6
65ab65c
bf62a8e
b9366b0
53b42e2
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,17 +1,16 @@ | ||
import * as React from 'react'; | ||
import axios from 'axios'; | ||
import { downFile } from '../../static/function'; | ||
import { | ||
Stack, PrimaryButton, DefaultButton, Panel, StackItem, Pivot, PivotItem | ||
} from 'office-ui-fabric-react'; | ||
import { MANAGER_IP, DRAWEROPTION } from '../../static/const'; | ||
import { DRAWEROPTION } from '../../static/const'; | ||
import { EXPERIMENT, TRIALS } from '../../static/datamodel'; | ||
import MonacoEditor from 'react-monaco-editor'; | ||
import '../../static/style/logDrawer.scss'; | ||
import { TrialManager } from '../../static/model/trialmanager'; | ||
|
||
interface ExpDrawerProps { | ||
isVisble: boolean; | ||
closeExpDrawer: () => void; | ||
experimentProfile: object; | ||
} | ||
|
||
interface ExpDrawerState { | ||
|
@@ -21,7 +20,9 @@ interface ExpDrawerState { | |
|
||
class ExperimentDrawer extends React.Component<ExpDrawerProps, ExpDrawerState> { | ||
|
||
public _isCompareMount!: boolean; | ||
public _isExperimentMount!: boolean; | ||
private refreshId!: number | undefined; | ||
|
||
constructor(props: ExpDrawerProps) { | ||
super(props); | ||
|
||
|
@@ -32,42 +33,40 @@ class ExperimentDrawer extends React.Component<ExpDrawerProps, ExpDrawerState> { | |
} | ||
|
||
getExperimentContent = (): void => { | ||
axios | ||
.all([ | ||
axios.get(`${MANAGER_IP}/experiment`), | ||
axios.get(`${MANAGER_IP}/trial-jobs`), | ||
axios.get(`${MANAGER_IP}/metric-data`) | ||
]) | ||
.then(axios.spread((resExperiment, resTrialJobs, resMetricData) => { | ||
if (resExperiment.status === 200 && resTrialJobs.status === 200 && resMetricData.status === 200) { | ||
if (resExperiment.data.params.searchSpace) { | ||
resExperiment.data.params.searchSpace = JSON.parse(resExperiment.data.params.searchSpace); | ||
} | ||
const trialMessagesArr = TrialManager.expandJobsToTrials(resTrialJobs.data); | ||
const interResultList = resMetricData.data; | ||
Object.keys(trialMessagesArr).map(item => { | ||
// not deal with trial's hyperParameters | ||
const trialId = trialMessagesArr[item].id; | ||
// add intermediate result message | ||
trialMessagesArr[item].intermediate = []; | ||
Object.keys(interResultList).map(key => { | ||
const interId = `${interResultList[key].trialJobId}-${interResultList[key].parameterId}`; | ||
if (trialId === interId) { | ||
trialMessagesArr[item].intermediate.push(interResultList[key]); | ||
} | ||
}); | ||
}); | ||
const result = { | ||
experimentParameters: resExperiment.data, | ||
trialMessage: trialMessagesArr | ||
}; | ||
if (this._isCompareMount === true) { | ||
this.setState({ experiment: JSON.stringify(result, null, 4) }); | ||
} | ||
const experimentData = JSON.parse(JSON.stringify(this.props.experimentProfile)); | ||
if (experimentData.params.searchSpace) { | ||
experimentData.params.searchSpace = JSON.parse(experimentData.params.searchSpace); | ||
} | ||
const trialMessagesArr = TRIALS.getTrialJobList(); | ||
const interResultList = TRIALS.getMetricsList(); | ||
Object.keys(trialMessagesArr).map(item => { | ||
// not deal with trial's hyperParameters | ||
const trialId = trialMessagesArr[item].id; | ||
// add intermediate result message | ||
trialMessagesArr[item].intermediate = []; | ||
Object.keys(interResultList).map(key => { | ||
const interId = `${interResultList[key].trialJobId}-${interResultList[key].parameterId}`; | ||
if (trialId === interId) { | ||
trialMessagesArr[item].intermediate.push(interResultList[key]); | ||
} | ||
})); | ||
} | ||
}); | ||
}); | ||
const result = { | ||
experimentParameters: experimentData, | ||
trialMessage: trialMessagesArr | ||
}; | ||
if (this._isExperimentMount === true) { | ||
this.setState({ experiment: JSON.stringify(result, null, 4) }); | ||
} | ||
|
||
if (['DONE', 'ERROR', 'STOPPED'].includes(EXPERIMENT.status)) { | ||
if(this.refreshId !== null || this.refreshId !== undefined){ | ||
window.clearInterval(this.refreshId); | ||
} | ||
} | ||
|
||
} | ||
|
||
downExperimentParameters = (): void => { | ||
const { experiment } = this.state; | ||
downFile(experiment, 'experiment.json'); | ||
|
@@ -78,31 +77,28 @@ class ExperimentDrawer extends React.Component<ExpDrawerProps, ExpDrawerState> { | |
} | ||
|
||
componentDidMount(): void { | ||
this._isCompareMount = true; | ||
this._isExperimentMount = true; | ||
this.getExperimentContent(); | ||
this.refreshId = window.setInterval(this.getExperimentContent, 10000); | ||
window.addEventListener('resize', this.onWindowResize); | ||
} | ||
|
||
componentWillReceiveProps(nextProps: ExpDrawerProps): void { | ||
const { isVisble } = nextProps; | ||
if (isVisble === true) { | ||
this.getExperimentContent(); | ||
} | ||
} | ||
|
||
componentWillUnmount(): void { | ||
this._isCompareMount = false; | ||
this._isExperimentMount = false; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Extra space before There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ok |
||
window.clearTimeout(this.refreshId); | ||
window.removeEventListener('resize', this.onWindowResize); | ||
} | ||
|
||
render(): React.ReactNode { | ||
const { isVisble, closeExpDrawer } = this.props; | ||
const { closeExpDrawer } = this.props; | ||
const { experiment, expDrawerHeight } = this.state; | ||
return ( | ||
<Stack className="logDrawer"> | ||
<Panel | ||
isOpen={isVisble} | ||
isOpen={true} | ||
hasCloseButton={false} | ||
isLightDismiss={true} | ||
onLightDismissClick={closeExpDrawer} | ||
styles={{ root: { height: expDrawerHeight, paddingTop: 15 } }} | ||
> | ||
<Pivot style={{ minHeight: 190 }} className="log-tab-body"> | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -5,13 +5,14 @@ import { | |
Stack, initializeIcons, StackItem, CommandBarButton, | ||
IContextualMenuProps, IStackTokens, IStackStyles | ||
} from 'office-ui-fabric-react'; | ||
import LogDrawer from './Modals/LogDrawer'; | ||
import ExperimentDrawer from './Modals/ExperimentDrawer'; | ||
import LogPanel from './Modals/LogPanel'; | ||
import ExperimentPanel from './Modals/ExperimentPanel'; | ||
import { | ||
downLoadIcon, infoIconAbout, | ||
timeIcon, disableUpdates, requency, closeTimer | ||
} from './Buttons/Icon'; | ||
import { OVERVIEWTABS, DETAILTABS, NNILOGO } from './stateless-component/NNItabs'; | ||
import { EXPERIMENT } from '../static/datamodel'; | ||
import '../static/style/nav/nav.scss'; | ||
import '../static/style/icon.scss'; | ||
|
||
|
@@ -97,9 +98,9 @@ class NavCon extends React.Component<NavProps, NavState> { | |
openDocs = (): void => { | ||
window.open(WEBUIDOC); | ||
} | ||
|
||
openGithubNNI = (): void => { | ||
const {version} = this.state; | ||
const { version } = this.state; | ||
const nniLink = `https://github.com/Microsoft/nni/tree/${version}`; | ||
window.open(nniLink); | ||
} | ||
|
@@ -178,8 +179,8 @@ class NavCon extends React.Component<NavProps, NavState> { | |
</Stack> | ||
</StackItem> | ||
{/* the drawer for dispatcher & nnimanager log message */} | ||
{isvisibleLogDrawer && <LogDrawer closeDrawer={this.closeLogDrawer} />} | ||
<ExperimentDrawer isVisble={isvisibleExperimentDrawer} closeExpDrawer={this.closeExpDrawer} /> | ||
{isvisibleLogDrawer && <LogPanel closeDrawer={this.closeLogDrawer} />} | ||
{isvisibleExperimentDrawer && <ExperimentPanel closeExpDrawer={this.closeExpDrawer} experimentProfile={EXPERIMENT.profile} />} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't know if it's proper to pass There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. had tested this status. ExperimentPanel will refresh as interval not every time. |
||
</Stack> | ||
); | ||
} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -48,6 +48,15 @@ class TrialManager { | |
private latestMetricdataErrorMessage: string = ''; // metric-data-latest error message | ||
private isMetricdataRangeError: boolean = false; // metric-data-range api error filed | ||
private metricdataRangeErrorMessage: string = ''; // metric-data-latest error message | ||
private metricsList: Array<any> = []; | ||
private trialJobList: Array<any> = []; | ||
|
||
public getMetricsList(): Array<any> { | ||
return this.metricsList; | ||
} | ||
public getTrialJobList(): Array<any> { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Trial job should have a type. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. had fixed. add type |
||
return this.trialJobList; | ||
} | ||
|
||
public async init(): Promise<void> { | ||
while (!this.infoInitialized || !this.metricInitialized) { | ||
|
@@ -58,7 +67,7 @@ class TrialManager { | |
} | ||
} | ||
|
||
public async update(lastTime?: boolean): Promise<boolean> { | ||
public async update(lastTime?: boolean): Promise<boolean | void> { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same here There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. had fixed. |
||
const [infoUpdated, metricUpdated] = await Promise.all([this.updateInfo(), this.updateMetrics(lastTime)]); | ||
return infoUpdated || metricUpdated; | ||
} | ||
|
@@ -230,6 +239,7 @@ class TrialManager { | |
requestAxios(`${MANAGER_IP}/trial-jobs`) | ||
.then(data => { | ||
const newTrials = TrialManager.expandJobsToTrials(data as any); | ||
this.trialJobList = newTrials; | ||
for (const trialInfo of newTrials as TrialJobInfo[]) { | ||
if (this.trials.has(trialInfo.id)) { | ||
// eslint-disable-next-line @typescript-eslint/no-non-null-assertion | ||
|
@@ -252,7 +262,8 @@ class TrialManager { | |
return updated; | ||
} | ||
|
||
private async updateMetrics(lastTime?: boolean): Promise<boolean> { | ||
private async updateMetrics(lastTime?: boolean): Promise<boolean | void> { | ||
// private async updateMetrics(lastTime?: boolean): Promise<boolean> { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Remove unused code. Why should it be There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. finally, use line 265. Thanks. |
||
if (this.trials.size < METRIC_GROUP_UPDATE_THRESHOLD || lastTime) { | ||
return await this.updateAllMetrics(); | ||
} else { | ||
|
@@ -263,9 +274,12 @@ class TrialManager { | |
} | ||
} | ||
|
||
private async updateAllMetrics(): Promise<boolean> { | ||
private async updateAllMetrics(): Promise<boolean | void> { | ||
return requestAxios(`${MANAGER_IP}/metric-data`) | ||
.then(data => this.doUpdateMetrics(data as any, false)) | ||
.then(data => { | ||
this.metricsList = data; | ||
this.doUpdateMetrics(data as any, false); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. return this.doUpdateMetrics There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. why return this function? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ok. will return this function |
||
}) | ||
.catch(error => { | ||
this.isMetricdataError = true; | ||
this.MetricdataErrorMessage = `${error.message}`; | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the id format has changed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
had updated.