-
Notifications
You must be signed in to change notification settings - Fork 1
Open
Description
There were previous attempts to integrate the https://github.com/DynareJulia/GeneralizedSylvesterSolver.jl but there was a bug in its usage.
The suspicon was that it was doing a loop and calling the sylvester solver inplace, but since the solver works "inplace" it might require adding new buffers or zeroing out buffers between calls in the iteration.
To do this:
- Ensure that the call in the main perturbation solution is doing the same thing as the MatrixEquations in https://github.com/HighDimensionalEconLab/DifferentiableStateSpaceModels.jl/blob/v0.4.19/src/generate_perturbation.jl#L338-L339
- The suspicion was that this was correct
- Ensure that the call to the first order perturbation solver is doing the same thing as in https://github.com/HighDimensionalEconLab/DifferentiableStateSpaceModels.jl/blob/v0.4.19/src/generate_perturbation_derivatives.jl#L143-L144
- This may or may not have been correct.
- But the issue may have been that that it was changing something inplace. Maybe the
buff.Eor something like that? - After checking which of those values gets modified inplace, you will need to make a copy of them before the loop there and then copy it back into the
buff.Eor whatever before calling the buffers.
- Same thing with the second order one in https://github.com/HighDimensionalEconLab/DifferentiableStateSpaceModels.jl/blob/v0.4.19/src/generate_perturbation_derivatives.jl#L144
- Again, if any of the arguments are modified inplace then you will need to make a copy of it before the loop over
n_pand the copy it back over top prior to the call to thegeneralized_sylvester_solver
- Again, if any of the arguments are modified inplace then you will need to make a copy of it before the loop over
- Make sure the unit tests are going to catch gradients on multiple parameters, and with a reused cache between the two calls.
If it is working well then
- Run the benchmarks
- Flip the default from
:MatrixEquationsto:GeneralizedSylvesterSolverin https://github.com/HighDimensionalEconLab/DifferentiableStateSpaceModels.jl/blob/v0.4.19/src/types.jl#L372 - Run the benchmarks
If it seems to win in the benchmarks version MatrixEquations and unit tetss pass, then leave that change in the toggle.
Reactions are currently unavailable
Metadata
Metadata
Assignees
Labels
No labels