Closed Osburg closed 5 months ago
Hi @Osburg, thank you very much. I try to have a look over the weekend. Best, Johannes
Hi @Osburg
I walked a bit through the DoE code and understood thereby also what was wrong with the gradients with my very simple implementation. Doing this, I had an idea which perhaps would yield a more easy solution:
What you are doing is to transform the problem aka the domain
object to use bounds between -1 and 1 for the continuous inputs. Doing this, you also have to transform the constraints which is easy for the linear ones and more complex for the nonlinears and would also require changes on the code base for the NChooseK
constraints, so you are transforming the complete optimization problem to bounds -1 and 1.
My idea would be to keep the actual optimization problem (solved by ipopt
) in the original domain but calculate the actual objective in the scaled one, this would imply the following steps for a given X
and the D-optimality criterion:
X
between -1 and 1 and receive X_s
. X_m
out of X_s
based on the modellogdet(X_m.T@X_m)
Regarding the jacobian, you get the outer one based one by torch autodiff and the inner one of X_m.T@X_m
with respect to X_s
by the help of symbolic differentiaion by formulaic and sympy. What would be currently missing is just the jacobian of X_s
with respect to X
which should be relatively easy to calculate or, via the chain rule it could then be combined with the rest.
What do you think? For me this sounds easier and more elegant as we get rid of the transformation of the domain.
Best,
Johannes
Hi @jduerholt,
thanks for having a look at this PR. I think you are right, scaling inside the objective is easier. We could give the Objective
class another member containing information about the trafo, change Objective.evaluate()
to sth like
def evaluate(x):
return _evaluate(self.trafo(x))
_evaluate()
would be a new abstract method defined just as evaluate()
is defined now. Same could be done with the gradient, the only difference is that we would have to additionally scale the result by some constant factors (the derivative of the trafo should be some const. diagonal matrix, right?). This way we fix the issue for all current and future objective classes. What do you think about this?
DoE
and generate stuff like the scaling trafo (and/or additional stuff that is unneccessarily done in find_local_max_ipopt()
) in there? find_local_max_ipopt()
is already so bloated (and i find design.py very messy in general :D Think I should tidy this up somewhen...), would like to avoid putting more stuff in this function...Cheers Aaron :)
Hi @Osburg,
fine for me with the new abstract method evaluate
. For the jacobian I would then setup something like:
def evaluate_jacobian(self, x):
return self._evaluate_jacobian(x) * self.trafo_jacobian()
As the new jacobian is just the old one multiplied with a constant matrix which just depends on the lower and upper bounds of the features and the lower and upper bound of the scaling operation, which was in this discussion always [-1,1]
.
One could think about setting up the transformation as an own class which one provides with the lower and upper bound in which the scaling should happen.
This would also be my preferred solution.
In general, I like the idea of structuring the code better but I would not introduce a new class DoE but would rather go for a tighter integration into bofire
by creating an abstract functional class from which then the actual DoEStrategy
and SpaceFillingStrategy
would inherit. What do you think?
Can you estimate, if you have time this week for a first working implementation of the scaling? I have a use case in which I want to try it, if not, I will give it a try for a first version.
Best,
Johannes
Hi @jduerholt ,
yes, will have time to do work on it no later than this weekend. + thanks for starting in #358.
Cheers Aaron :)
This PR is a draft for an implementation of multi-start doe as described in #343 and for optional scaling of decision variables to (-1,1). Closes #343 and #351.