Initial analytical gradient implementation #110
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
|
||
| for (std::size_t k = 0; k < idxPsi.n_elem; ++k) { | ||
| const std::size_t pos = idxPsi[k]; | ||
| grad[pos] = gradPsi(row[pos], col[pos]); | ||
| } |
There was a problem hiding this comment.
The analytic branch fills the gradient vector by reading a single matrix entry, e.g. grad[pos] = gradPsi(row[pos], col[pos]);, without taking symmetric[pos] into account. For blocks like psi, thetaDelta and omegaXiXi the optimiser treats each off‑diagonal parameter as a single value that simultaneously perturbs (i,j) and (j,i) (the finite‑difference fallback does this by updating both entries). Returning only the derivative for one matrix entry underestimates the directional derivative by a factor of two for off‑diagonal parameters, so gradients are inconsistent with the parameterization and can mislead gradient‑based optimisation whenever covariance matrices have free off‑diagonal elements. The analytic mapping should combine the symmetric pair before writing to grad.
Useful? React with 👍 / 👎.
No description provided.