You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
I encountered a bug() when trying to define a LeastSquaresProblem from tuples when the objective tuples consist of a mix of 1d-array objectives and scalar objectives.
ValueErrorTraceback (mostrecentcalllast)
CellIn[1], line1916list_opt=make_optimizable(return_list, singleton2)
17obj_tuple2= (list_opt.J, np.ones(3), 1)
--->19prob=LeastSquaresProblem.from_tuples([obj_tuple1, obj_tuple2])
21least_squares_serial_solve(prob)
File~/code/simsopt/src/simsopt/objectives/least_squares.py:136, inLeastSquaresProblem.from_tuples(cls, tuples, fail)
127""" 128 Initializes graph based LeastSquaresProblem from a sequence of tuples 129 containing *f_in*, *goal*, and *weight*. (...) 133 each tuple (the specified order matters). 134 """135funcs_in, goals, weights=zip(*tuples)
-->136returncls(goals, weights, funcs_in=funcs_in, fail=fail)
File~/code/simsopt/src/simsopt/objectives/least_squares.py:68, inLeastSquaresProblem.__init__(self, goals, weights, funcs_in, depends_on, opt_return_fns, fail)
66ifnp.any(np.asarray(weights) <0):
67raiseValueError('Weight cannot be negative')
--->68self.goals=np.asarray(goals)
69self.inp_weights=np.asarray(weights)
70self.fail=fail
This is because LeastSquaresProblem.from_tuples zips the inputs, and np.asarray receives a mix of integers and lists.
This could be solved by ravel-concatenating the inputs in from_tuples:
@classmethoddeffrom_tuples(cls,
tuples: Sequence[Tuple[Callable, Real, Real]],
fail: Union[None, float] =1.0e12) ->LeastSquaresProblem:
""" Initializes graph based LeastSquaresProblem from a sequence of tuples containing *f_in*, *goal*, and *weight*. Args: tuples: A sequence of tuples containing (f_in, goal, weight) in each tuple (the specified order matters). """funcs_in, goals, weights=zip(*tuples)
goal_array=np.concatenate([np.ravel(x) forxingoals])
weigth_array=np.concatenate([np.ravel(x) forxinweights])
returncls(goal_array, weigth_array, funcs_in=funcs_in, fail=fail)
The reason this is an issue, and not a pull-request, is that I don't know if the weight-arrays and goal-arrays are still correctly handled after this change. I don't see the code that expands the weights array to the same size of the goals array, but on the other hand, from_tuples only worked if weight was an integer, even if goals and obj_return.J() were arrays (a formatting function would fail in the argument to an objective_file.write call).
So: Should weight_array be ravel-concatenated as well? And should the user be able to specify weight as an array when the goal is an array? Are the weights still correctly assigned to the goals when mixed input is provided?
[also as an aside, Identity and some other Optimizable classes are defined with a .f() evaluation function, wheras the standard int he rest of simsopt seems to be .J(). Would it be good to re-factor this?]
The text was updated successfully, but these errors were encountered:
I encountered a bug() when trying to define a LeastSquaresProblem from tuples when the objective tuples consist of a mix of 1d-array objectives and scalar objectives.
Below is a MWE:
This throws the ValueError:
This is because
LeastSquaresProblem.from_tuples
zips the inputs, andnp.asarray
receives a mix of integers and lists.This could be solved by
ravel
-concatenating
the inputs infrom_tuples
:The reason this is an issue, and not a pull-request, is that I don't know if the
weight
-arrays andgoal
-arrays are still correctly handled after this change. I don't see the code that expands theweights
array to the same size of thegoals
array, but on the other hand,from_tuples
only worked if weight was an integer, even ifgoals
andobj_return.J()
were arrays (a formatting function would fail in the argument to anobjective_file.write
call).So: Should
weight_array
beravel
-concatenated
as well? And should the user be able to specifyweight
as an array when the goal is an array? Are theweights
still correctly assigned to the goals when mixed input is provided?[also as an aside,
Identity
and some otherOptimizable
classes are defined with a.f()
evaluation function, wheras the standard int he rest of simsopt seems to be.J()
. Would it be good to re-factor this?]The text was updated successfully, but these errors were encountered: