search

SciML / Optimization.jl

Mathematical Optimization in Julia. Local, global, gradient-based and derivative-free. Linear, Quadratic, Convex, Mixed-Integer, and Nonlinear Optimization in one simple, fast, and differentiable interface.
https://docs.sciml.ai/Optimization/stable/
MIT License
688 stars 75 forks source link

Reversion: stop deep copying population, just save `curr_u` for `OptimizationState` construction #734

Closed jonathanfischer97 closed 2 months ago

jonathanfischer97 commented 2 months ago

  1. Regarding my previous PRs #702 and #732, I think they were mistaken because of my own unclear thinking about why dt["x"] needed to be saved via the default Evolutionary.trace! overload.

  2. I was operating from the perspective of my own uncommon workflow, where I accumulate "feasible" optimization solutions with each iteration, so I overload the trace! to save deep copies of all these feasible solutions, so that the trace acts as a permanent record.

  3. However, this is likely much different from the typical use case, where only the most recent OptimizationState is relevant for any user-injected callback. So a reference to curr_u as it is used in the callback interface _cb is fine, and in-fact preferred in most cases to minimize memory footprint.

  4. Therefore, I have changed the Evolutionary.trace! overload to just save a reference the last element of the population, under the key "curr_u", in line with how the trace was previously being accessed within _cb. This also clarifies the code by making the reason for the trace! overload more transparent.

  5. Any further trace functionality that a user might need, like in my own use case, should be left for the user to define via trace! overload. This keeps this package and its default settings tailored to the typical use case.

Apologies, should have thought more carefully before making the previous PRs.

codecov[bot] commented 2 months ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 63.85%. Comparing base (e0610cd) to head (e2cad3d). Report is 15 commits behind head on master.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #734 +/- ## =========================================== + Coverage 11.37% 63.85% +52.47% =========================================== Files 22 22 Lines 1512 1527 +15 =========================================== + Hits 172 975 +803 + Misses 1340 552 -788 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

Vaibhavdixit02 commented 2 months ago

No worries, this makes sense