-
Notifications
You must be signed in to change notification settings - Fork 137
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
Mandd/pareto front PP prestruct #1479
Conversation
@@ -22,7 +22,7 @@ | |||
|
|||
|
|||
|
|||
def nonDominatedFrontier(data, returnMask): | |||
def nonDominatedFrontier(data, returnMask, minMask=None): |
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.
@wangcj05 I honestly feel like the required structure of returnMask and minMask might not be optimal, if you have comments/suggestions let me know
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.
Sorry, I did not get your point here. Could you clarify it?
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 was not sure if passing the data as numpy array and min mask as a separate numpy array was the best way to pass this info to this method
for child in paramInput.subparts: | ||
if child.getName() == 'objective': | ||
self.objectives[child.value]={} | ||
if child.parameterValues['goal'] in ['min','max']: |
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.
You can use InputTypes.makeEnumType in getInputSpecification to handle this, in that case, you do not need to check the goal input here.
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.
good point: added enumType and removed checks
self.raiseAnError(IOError, 'ParetoFrontier postprocessor {}: the objective {} expects a min/max goal, but received {} inputs!".' | ||
.format(self.name,child.value,child.parameterValues['goal'])) | ||
if 'upperLimit' in child.parameterValues.keys(): | ||
self.objectives[child.value]['upperLimit'] = child.parameterValues['upperLimit'] |
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.
Maybe you can use child.parameterValues.get('upperLimit')? Default is None if not available
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.
edited
if 'upperLimit' in child.parameterValues.keys(): | ||
self.objectives[child.value]['upperLimit'] = child.parameterValues['upperLimit'] | ||
if 'lowerLimit' in child.parameterValues.keys(): | ||
self.objectives[child.value]['lowerLimit'] = child.parameterValues['lowerLimit'] |
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 above comment
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.
edited
selection = data.isel(RAVEN_sample_ID=np.array(paretoFrontMask)) | ||
|
||
for obj in self.objectives.keys(): | ||
if 'upperLimit' in self.objectives[obj].keys(): |
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.
If you make changes according to previous comments, this line can be changed:
if self.objectives[obj]['upperLimit']:
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.
yup, code is cleaner
if 'upperLimit' in self.objectives[obj].keys(): | ||
selection = selection.where(selection[obj]<=self.objectives[obj]['upperLimit']) | ||
if 'lowerLimit' in self.objectives[obj].keys(): | ||
selection = selection.where(selection[obj]>=self.objectives[obj]['lowerLimit']) |
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.
same here
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.
edited as well
framework/utils/frontUtils.py
Outdated
if minMask is None: | ||
pass | ||
elif minMask is not None and minMask.shape[0] != data.shape[1]: | ||
raise IOError("nonDominatedFrontier method: minMask has shape " + str(data.shape) + " while minMask has shape " + str(minMask.shape)) |
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.
Update the error message, it seems not clear to me
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.
message expanded and fixed
@@ -22,7 +22,7 @@ | |||
|
|||
|
|||
|
|||
def nonDominatedFrontier(data, returnMask): | |||
def nonDominatedFrontier(data, returnMask, minMask=None): |
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.
Sorry, I did not get your point here. Could you clarify it?
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.
Addressing comments
for child in paramInput.subparts: | ||
if child.getName() == 'objective': | ||
self.objectives[child.value]={} | ||
if child.parameterValues['goal'] in ['min','max']: |
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.
good point: added enumType and removed checks
self.raiseAnError(IOError, 'ParetoFrontier postprocessor {}: the objective {} expects a min/max goal, but received {} inputs!".' | ||
.format(self.name,child.value,child.parameterValues['goal'])) | ||
if 'upperLimit' in child.parameterValues.keys(): | ||
self.objectives[child.value]['upperLimit'] = child.parameterValues['upperLimit'] |
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.
edited
if 'upperLimit' in child.parameterValues.keys(): | ||
self.objectives[child.value]['upperLimit'] = child.parameterValues['upperLimit'] | ||
if 'lowerLimit' in child.parameterValues.keys(): | ||
self.objectives[child.value]['lowerLimit'] = child.parameterValues['lowerLimit'] |
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.
edited
selection = data.isel(RAVEN_sample_ID=np.array(paretoFrontMask)) | ||
|
||
for obj in self.objectives.keys(): | ||
if 'upperLimit' in self.objectives[obj].keys(): |
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.
yup, code is cleaner
if 'upperLimit' in self.objectives[obj].keys(): | ||
selection = selection.where(selection[obj]<=self.objectives[obj]['upperLimit']) | ||
if 'lowerLimit' in self.objectives[obj].keys(): | ||
selection = selection.where(selection[obj]>=self.objectives[obj]['lowerLimit']) |
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.
edited as well
@@ -22,7 +22,7 @@ | |||
|
|||
|
|||
|
|||
def nonDominatedFrontier(data, returnMask): | |||
def nonDominatedFrontier(data, returnMask, minMask=None): |
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 was not sure if passing the data as numpy array and min mask as a separate numpy array was the best way to pass this info to this method
framework/utils/frontUtils.py
Outdated
if minMask is None: | ||
pass | ||
elif minMask is not None and minMask.shape[0] != data.shape[1]: | ||
raise IOError("nonDominatedFrontier method: minMask has shape " + str(data.shape) + " while minMask has shape " + str(minMask.shape)) |
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.
message expanded and fixed
@mandd please update the user manual regarding the input syntax changes made in this PR. |
…olab/raven into mandd/paretoFrontPPrestruct
Job Test mac on 1cedcba : invalidated by @joshua-cogliati-inl upgraded miniconda, trying again |
Job Test mac on 1cedcba : invalidated by @joshua-cogliati-inl restarted civet |
Pull Request Description
What issue does this change request address? (Use "#" before the issue to link it, i.e., #42.)
The current Pareto front PP can handle 2-dimensional multi-objective optimization
This PR extends PP capabilities to multiple dimensions optimization
Issue #1448 #1447
What are the significant changes in functionality due to this change request?
For Change Control Board: Change Request Review
The following review must be completed by an authorized member of the Change Control Board.
<internalParallel>
to True.raven/tests/framework/user_guide
andraven/docs/workshop
) have been changed, the associated documentation must be reviewed and assured the text matches the example.