Improve human feedback UI for Preferential Optimization #572
Conversation
| fontWeight: theme.typography.fontWeightBold, | ||
| }} | ||
| > | ||
| Which trial is worst? |
There was a problem hiding this comment.
| Which trial is worst? | |
| Which trial is the worst? |
| }} | ||
| /> | ||
| ))} | ||
| <Box> |
There was a problem hiding this comment.
I think you should put some paddings inside the content.
| const shuffleFlag = shuffleTrial + 20 < displayTrials.last_number | ||
| if (shuffleFlag) { | ||
| setShuffleTrial(displayTrials.last_number) | ||
| } |
There was a problem hiding this comment.
Could you explain the purpose of this?
There was a problem hiding this comment.
Sorry, it is mistaken. This is a trial function that avoids giving bias due to position.
| @@ -12,40 +17,108 @@ const PreferentialTrial: FC<{ | |||
| const theme = useTheme() | |||
| const action = actionCreator() | |||
| const trialWidth = 500 | |||
| const trialHeight = 300 | |||
| const [hover, setHover] = useState(false) | |||
There was a problem hiding this comment.
Could you please avoid to use state for the performance reason? We should write the animation in CSS.
c-bata
changed the title
| <Modal open={detailShown} onClose={() => setDetailShown(false)}> | ||
| <Box | ||
| sx={{ | ||
| position: "relative", | ||
| width: "70%", | ||
| left: "15%", | ||
| top: theme.spacing(8), | ||
| backgroundColor: theme.palette.mode === "dark" ? "black" : "white", | ||
| }} | ||
| > | ||
| <TrialListDetail | ||
| trial={trial} | ||
| isBestTrial={() => false} | ||
| directions={[]} | ||
| objectiveNames={[]} | ||
| /> | ||
| </Box> | ||
| </Modal> |
There was a problem hiding this comment.
Could you update the styles as the following PR sets borderRadius and uses position: "absolute"?
#552
|
Thank you for reviewing. I fixed them. |
| position: "absolute", | ||
| width: "70%", | ||
| left: "15%", | ||
| top: theme.spacing(8), |
There was a problem hiding this comment.
| position: "absolute", | |
| width: "70%", | |
| left: "15%", | |
| top: theme.spacing(8), | |
| position: "absolute", | |
| top: 0, | |
| left: 0, | |
| right: 0, | |
| bottom: 0, | |
| width: "80%", | |
| maxHeight: "90%", | |
| margin: "auto", |
How about positioning to the center like this?
| top: 0, | ||
| left: 0, | ||
| color: "red", | ||
| fontSize: trialWidth / 2, |
There was a problem hiding this comment.
Is there something effect on this? I removed this line, but cannot see any changes actually.
There was a problem hiding this comment.
The "fontSize" does not affect it, so I erased it.
However, this icon did not appear when I tried to remove any other.
| sx={{ | ||
| width: trialWidth, | ||
| minHeight: trialHeight, | ||
| position: "relative", |
There was a problem hiding this comment.
Could you explain why this component should be position: relative?
Contributor License Agreement
This repository (
optuna-dashboard) and Goptuna share common code.This pull request may therefore be ported to Goptuna.
Make sure that you understand the consequences concerning licenses and check the box below if you accept the term before creating this pull request.
Reference Issues/PRs
What does this implement/fix? Explain your changes.