SimulatedAnnealing Discrete Variables #2312
Conversation
|
Job Precheck on a794170 : invalidated by @joshua-cogliati-inl failed because of hpcgitlab being down |
alfoa
changed the title
…m:idaholab/raven into alfoa/simulatedAnnealingDiscreteVariables
|
@Jimmy-INL @wangcj05 this is ready for review |
There was a problem hiding this comment.
@alfoa I have a couple of comments for you to consider. Thanks for the updates regarding the normalized and unnormalized data. It is really confusing in our current code. Some suggestions here:
- It seems some of the discrete handling can be moved to Optimizer base class
- We need a better document for the data we store in the self. variables. For example, self._optPointHistory stores the normalized data. it is really not clear when our 'rlz' are normalized or not. Again, thanks for you update of our docstring to make it more clear.
| @@ -1614,6 +1622,11 @@ def getInputSpecification(cls): | |||
| StatePartInput = InputData.parameterInputFactory("state", contentType=InputTypes.FloatType) | |||
| StatePartInput.addParam("outcome", InputTypes.FloatOrStringType, True) | |||
| inputSpecification.addSub(StatePartInput, InputData.Quantity.one_to_infinity) | |||
| inputSpecification.addSub(InputData.parameterInputFactory("rtol", | |||
There was a problem hiding this comment.
Please update user manual to reflect it.
| val = init[var] | ||
| values[var] = self.distDict[var].ppf(self.distDict[var].cdf(val)) | ||
| if not np.isclose(initialValues[traj][var], values[var], 1e-4): | ||
| self.raiseAWarning(f"Traj: {traj}. Variable {var} is associated with a discrete distribution. The inputted initial value {initialValues[traj][var]} " |
There was a problem hiding this comment.
maybe define a general tolerance variable for 1e-4.
There was a problem hiding this comment.
Using the utils.isClose now
|
|
||
| initialValues = self._initialValues | ||
| for traj, init in enumerate(self._initialValues): | ||
| self._submitRun(init,traj,self.getIteration(traj)) | ||
| values = {} | ||
| init = self.denormalizeData(init) | ||
| initialValues[traj] = self.denormalizeData(initialValues[traj]) | ||
| for var in init: | ||
| if var in self.toBeSampled and self.distDict[var].distType == distType.discrete: | ||
| val = init[var] | ||
| values[var] = self.distDict[var].ppf(self.distDict[var].cdf(val)) | ||
| if not np.isclose(initialValues[traj][var], values[var], 1e-4): | ||
| self.raiseAWarning(f"Traj: {traj}. Variable {var} is associated with a discrete distribution. The inputted initial value {initialValues[traj][var]} " | ||
| f"is not among available discrete values. Closest value is {values[var]}") | ||
| else: | ||
| values[var] = init[var] | ||
| values = self.normalizeData(values) | ||
|
|
||
| self._submitRun(values,traj,self.getIteration(traj)) |
There was a problem hiding this comment.
It seems to me these lines can be moved to Optimizer base class.
There was a problem hiding this comment.
Agree. This probably would require a general "re-design" of the optimizer for discrete variables (with exceptions for not-discrete optimizers (such as the gradient descend))
There was a problem hiding this comment.
I would open an issue to track this in case the RAVEN team is willing to go in that direction
| # queue up the first run for each trajectory | ||
| #for traj, init in enumerate(self._initialValues): | ||
| # self._submitRun(init,traj,self.getIteration(traj)) |
| nextNeighbour[var] = rlzNormalized[var] + delta[i] | ||
| nextNeighbour[var] = 0 if nextNeighbour[var] < 0 else nextNeighbour[var] | ||
| nextNeighbour[var] = 1 if nextNeighbour[var] > 1 else nextNeighbour[var] | ||
| if self.distDict[var].distType == distType.discrete: | ||
| val = nextNeighbour[var] | ||
| nextNeighbour[var] = self.distDict[var].cdf(self.distDict[var].ppf(val)) |
There was a problem hiding this comment.
Do you need to denormalize the data before you compute ppf?
There was a problem hiding this comment.
Theoretically the normalization is performed between 0-1 and it implicitly maps to a uniform distribution. I can add the denormalization in order to avoid that, if in the future the normalization strategy got changed, this would result in a diff
| <d>1.0</d> | ||
| </cauchy> | ||
| <veryfast> | ||
| <c>0.5</c> |
There was a problem hiding this comment.
modifed to reflect the test name "VeryFast" was supposed to be tested here
|
Job Mingw Test on de67311 : invalidated by @alfoa fetch failure |
|
Job Test qsubs sawtooth on de67311 : invalidated by @alfoa |
| <name>framework/Optimizers/SimulatedAnnealing/discrete.FuncionallyConstrainedSA</name> | ||
| <author>MohammadAbdo</author> | ||
| <created>2020-02-05</created> | ||
| <classesTested>Optimizer</classesTested> | ||
| <description> | ||
| This test uses a multidimensional linear function such that the trajectory must pass through | ||
| a functional constraint to reach the optimal point. | ||
| The test is designed by @talbpaul and is applied to simulated annealing. | ||
| </description> |
There was a problem hiding this comment.
Could you update the test description here.
.../SimulatedAnnealing/discrete/FunctionallyConstrainedSA/test_funcConstrSimulatedAnnealing.xml
Outdated
Show resolved
Hide resolved
…nallyConstrainedSA/test_funcConstrSimulatedAnnealing.xml
|
Job Mingw Test on 44192fa : invalidated by @wangcj05 |
|
Changes are good. PR checklists are good |
Pull Request Description
What issue does this change request address? (Use "#" before the issue to link it, i.e., #42.)
Closes #2305
What are the significant changes in functionality due to this change request?
Addition of the support for simulated annealing of discrete variables.
In order to minimize the modifications, the discrete variables are treated using cdf sampling and inverseCDF transformation.
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_guideandraven/docs/workshop) have been changed, the associated documentation must be reviewed and assured the text matches the example.