-
-
Notifications
You must be signed in to change notification settings - Fork 95
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fast pareto-front calculation for 2D #687
Merged
Merged
Changes from 3 commits
Commits
Show all changes
14 commits
Select commit
Hold shift + click to select a range
bd69232
Fast pareto-front calculation for 2D
contramundum53 e3c1d74
Fix bug in getIsDominatedTrialND
contramundum53 aba648e
Code fix
contramundum53 becaff1
Update optuna_dashboard/ts/components/GraphParetoFront.tsx
contramundum53 4ef4aa1
Update optuna_dashboard/ts/components/GraphParetoFront.tsx
contramundum53 b0f66b3
Update optuna_dashboard/ts/components/GraphParetoFront.tsx
contramundum53 5b4f0bb
Update optuna_dashboard/ts/components/GraphParetoFront.tsx
contramundum53 8ebed9b
fix pareto-front
contramundum53 422ffd2
rename variables
contramundum53 01ecc55
rename functions
contramundum53 a9b23ae
remove unnecessary condition
contramundum53 29d44ab
Fix bug
contramundum53 4315c9d
Update optuna_dashboard/ts/components/GraphParetoFront.tsx
contramundum53 1203501
Rename variable
contramundum53 File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -165,6 +165,58 @@ const makeMarker = ( | |||||
} | ||||||
} | ||||||
|
||||||
const getIsDominatedTrialND = (normalizedValues: number[][]) => { | ||||||
// Fallback for straight-forward pareto front algorithm (O(N^2) complexity). | ||||||
const dominatedTrials: boolean[] = [] | ||||||
normalizedValues.forEach((values0: number[]) => { | ||||||
const dominated = normalizedValues.some((values1: number[]) => { | ||||||
if (values0.every((value0: number, k: number) => values1[k] === value0)) { | ||||||
return false | ||||||
} | ||||||
return values0.every((value0: number, k: number) => values1[k] <= value0) | ||||||
}) | ||||||
dominatedTrials.push(dominated) | ||||||
}) | ||||||
return dominatedTrials | ||||||
} | ||||||
|
||||||
const getIsDominatedTrial2D = (normalizedValues: number[][]) => { | ||||||
// Fast pareto front algorithm (O(N log N) complexity). | ||||||
const sorted = normalizedValues | ||||||
.map((values, i) => [values[0], values[1], i]) | ||||||
.sort((a, b) => a[0] - b[0]) | ||||||
let minValue1 = sorted[0][1] | ||||||
const dominatedTrials: boolean[] = new Array(normalizedValues.length).fill( | ||||||
true | ||||||
) | ||||||
|
||||||
sorted.forEach((values) => { | ||||||
if (values[1] <= minValue1) { | ||||||
dominatedTrials[values[2]] = false | ||||||
minValue1 = values[1] | ||||||
} | ||||||
}) | ||||||
return dominatedTrials | ||||||
} | ||||||
HideakiImamura marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||
|
||||||
const getIsDominatedTrial1D = (normalizedValues: number[][]) => { | ||||||
const best_value = Math.min(...normalizedValues.map((values) => values[0])) | ||||||
return normalizedValues.map((value) => value[0] !== best_value) | ||||||
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.
Suggested change
|
||||||
} | ||||||
|
||||||
const getIsDominatedTrial = (normalizedValues: number[][]) => { | ||||||
if (normalizedValues.length == 0) { | ||||||
contramundum53 marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||
return [] | ||||||
} | ||||||
if (normalizedValues[0].length == 1) { | ||||||
contramundum53 marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||
return getIsDominatedTrial1D(normalizedValues) | ||||||
} else if (normalizedValues[0].length == 2) { | ||||||
contramundum53 marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||
return getIsDominatedTrial2D(normalizedValues) | ||||||
} else { | ||||||
return getIsDominatedTrialND(normalizedValues) | ||||||
} | ||||||
} | ||||||
|
||||||
const plotParetoFront = ( | ||||||
study: StudyDetail, | ||||||
objectiveXId: number, | ||||||
|
@@ -218,18 +270,7 @@ const plotParetoFront = ( | |||||
} | ||||||
}) | ||||||
|
||||||
const dominatedTrials: boolean[] = [] | ||||||
normalizedValues.forEach((values0: number[], i: number) => { | ||||||
const dominated = normalizedValues.some((values1: number[], j: number) => { | ||||||
if (i === j) { | ||||||
return false | ||||||
} | ||||||
return values0.every((value0: number, k: number) => { | ||||||
return values1[k] <= value0 | ||||||
}) | ||||||
}) | ||||||
dominatedTrials.push(dominated) | ||||||
}) | ||||||
const dominatedTrials: boolean[] = getIsDominatedTrial(normalizedValues) | ||||||
|
||||||
const plotData: Partial<plotly.PlotData>[] = [ | ||||||
makeScatterObject( | ||||||
|
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
This condition is buggy when the values between trials are the same.
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.
In that case, both trials should be considered undominated.
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.
Now given a sequence of 2D vectors$\{(a_n, b_n)\}_{n=1}^N$ such that $a_1 \leq a_2 \leq \dots \leq a_N$ and $b_i \leq b_j$ for all $\{(i, j) \in [N] \times [N] \mid a_i = a_j, i < j\}$ where $[N] \coloneqq \{1,2,\dots,N\}$ , the condition here is a bit strange when the equality holds.
For example,$a_i \leq a_j$ holds from the assumption and we now assume that $b_i = b_j$ .$a_i = a_j$ and $b_i = b_j$ do not dominate each other.$a_i < a_j$ and $\min_{n \in [i]} b_i = b_j$ could still hold.$(a_i, b_i)$ such that $a_i < a_j$ and $b_i \leq b_j$ , which weakly dominates $(a_j, b_j)$ .
As mentioned above, let's assume that
However,
It means that there could exist
Btw, I guessed you are trying to get "weak dominance" from
getIsDominatedTrialND
.Maybe I am wrong, if so, correct me please!
Output
I guess (if we need weakly dominated trials) what we want is:
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.
Quick solution to this is the following:
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.
See https://github.com/optuna/optuna/blob/master/optuna/study/_multi_objective.py#L101. Optuna uses
weakly dominates
for comparison. It seems that @nabenabe0928 is correct.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.
Thank you for pointing out. My statement is incorrect and @nabenabe0928's one is correct, I think.
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.
The new modification fails with the following case.
We get:
But we would like to have:
I think the suggestion here is sufficient.