fixed user trace! overload compat. Both x and user records now saved

[jonathanfischer97]

Checklist

• Appropriate tests were added
• Any code changes were done in a way that does not break public API
• All documentation related to code changes were updated
• The new code follows the
  contributor guidelines, in particular the SciML Style Guide and
  COLPRAC.
• Any new documentation only uses public API

Additional context

1. This PR improves compatibility with Evolutionary.jl trace! functionality. Specifically, Evolutionary.jl allows user-defined overloads of trace! in order to save other values during optimization, via an generic interface. This functionality is advertised within the Evolutionary.jl docs, seen here:

2. When using OptimizationEvolutionary however, this functionality was broken. Previously, any user overloads of trace! would be dispatched in-place of the existing overload in OptimizationEvolutionary, allowing the possibility of "x" not being saved and thus throwing an error during construction of OptimizationState.

# Previous overload in order to make sure `population` was saved
function Evolutionary.trace!(record::Dict{String, Any}, objfun, state, population,
        method::Evolutionary.AbstractOptimizer, options)
    record["x"] = population
end

3. To fix this, I've instead replaced the previous overload with an overload of the internal Evolutionary.trace! function, which calls the exposed trace! function. This overload adds "x" to the existing "time" key which will always be saved, prior to calling user methods and regardless of their existence.

# Overload the trace! function to add the population to the trace prior to calling any user-defined trace! method
function Evolutionary.trace!(tr, iteration, objfun, state, population, method::Evolutionary.AbstractOptimizer, options, curr_time=time()) 
    dt = Dict{String,Any}()
    dt["time"] = curr_time

    # record `x` to store the population. Needed for constructing OptimizationState.
    dt["x"] = population

    # set additional trace value
    Evolutionary.trace!(dt, objfun, state, population, method, options)
    Evolutionary.update!(tr,
            state,
            iteration,
            Evolutionary.value(state),
            dt,
            options.store_trace,
            options.show_trace,
            options.show_every,
            options.callback)
end

4. The added test tests that "x" as well as example user-defined trace record key are present in the output, which passed.

[codecov]

Codecov Report

Attention: 6 lines in your changes are missing coverage. Please review.

Comparison is base (fedb001) 8.69% compared to head (cf6caa2) 8.68%.

Files	Patch %	Lines
...zationEvolutionary/src/OptimizationEvolutionary.jl	0.00%	6 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff            @@
##           master    #702      +/-   ##
=========================================
- Coverage    8.69%   8.68%   -0.02%     
=========================================
  Files          31      31              
  Lines        2496    2500       +4     
=========================================
  Hits          217     217              
- Misses       2279    2283       +4     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[Vaibhavdixit02]

This is great. I'd want a more first class integration with the callback and OptimizationState things in the package for this but that can be done later. Thanks!

[jonathanfischer97]

@Vaibhavdixit02 Agreed, just wanted to keep the changes limited for this PR. No problem!

[Vaibhavdixit02]

Why did you close this @jonathanfischer97?

[jonathanfischer97]

@Vaibhavdixit02 Oops, my bad, thought branch was merged. Still getting the hang of contributing to public repos!

[Vaibhavdixit02]

OptimizationEvolutionary test failure looks real, can you take a look at it?

[jonathanfischer97]

@Vaibhavdixit02 Will do!

[jonathanfischer97]

@Vaibhavdixit02 Fixed the trace test, previous test was querying the trace of sol which wasn't storing trace higher up in the testset. So now sol is the solution from the solve testing the callback and trace functionality. Should pass now!

[Vaibhavdixit02]

Thanks, this is great!