Add optimization direction to invdes.Optimizer

[yaugenst-flex]

@e-g-melo

[tylerflex]

Looks good. Just wondering about naming.. direction could be confused with eg ModeSource.direction. Is there a better option? Perhaps "max_or_min", sign (-1,1), or maximize:bool?

[yaugenst-flex]

Yeah not sure. An alternative would be to do it the pytorch way and make it a maximize: bool = True argument. the explicit "maximize" vs "minimize" is a bit unnecessary because these values are not actually used.
nlopt has set_{min,max}_objective, but i think this is not feasible for us.
i would like to keep it explicit though by having either "maximize" or "minimize" in the variable name, otherwise people might get confused as to which is the default direction for a gradient 😄

[yaugenst-flex]

how about we go the boolean route?

[tylerflex]

Agree with everything. And let's just do Boolean. Only thing i can think of is are we ever going to want to support something other than minimizing or maximizing? Can't think of anything now so maybe let's just go with bool

[tylerflex]

There's one caveat to discuss. Penalties should always be minimized? If so then we want to apply the sign to the postprocess function value only. (In the objective function def)

[yaugenst-flex]

updated it by passing maximize to make_objective_fn(). i agree in terms of the code it would be cleaner if it was just defined in InverseDesign, but conceptually i don't see how a maximize argument makes sense in InverseDesign. it really belongs with the optimizer. and doing it via the function value instead of the gradient is also...not optimal 😄
but yeah i guess this would work for now?

[tylerflex]

hm, if

direction_multiplier = 1 if maximize else -1
obj_fn_val = direction_multiplier * post_process_val - penalty_value

then shouldn't we be using -grad here since we're always trying to maximize this quantity?

[yaugenst-flex]

oops

[yaugenst-flex]

fixed

[tylerflex]

not sure it's pushed? just fyi