Conversation
…ts signature to include path_alpha for improved directional derivative calculations
…ter and optimize kernel value calculation for improved clarity and performance
Contributor
There was a problem hiding this comment.
Pull request overview
This PR updates the L-BFGS line-search directional-derivative computation (field_dot_global) to better reflect the physical model update rules (multiplicative vs/vp/rho/gamma vs additive gc/gs) and adds more detailed logging/comments, aiming to address the stalling reported in issue #28.
Changes:
- Replaces the previous simple inner-product reduction with an MPI-reduced directional derivative that includes spherical surface-area quadrature weights.
- Adds chain-rule handling notes and additional debug logging for Wolfe-condition evaluation.
- Introduces a helper for trapezoidal nodal quadrature widths in lon/lat.
Comments suppressed due to low confidence (1)
src/optimize.cpp:139
- For radial anisotropy, the
p==5kernel is applied tovsh = vs*gamma(Love waves), but the code currently uses(vsh/vs) * dir[5], which corresponds togamma * d_gammaand misses thevsfactor needed for-d(vsh)/dalpha = vsh0 * d_gammaunder the multiplicative gamma update (gamma *= (1 - alpha*dir[5])). This will under-scaleq/q1and can break Wolfe line search decisions.
} else if (p == 5 &&
IP.inversion().model_para_type == MODEL_RADIAL_ANI) {
minus_dm_dalpha = (mg.vsh3d[index] / mg.vs3d[index]) * direction[p](ix, iy, iz);
} else {
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+121
to
+127
| if (p == 0) { | ||
| // vs(alpha) = vs0 * (1 - alpha*d_vs) | ||
| minus_dm_dalpha = | ||
| mg.vs3d[index] * direction[p](ix, iy, iz); | ||
| if (IP.inversion().model_para_type == MODEL_RADIAL_ANI) { | ||
| kernel_val = kernel[0](ix, iy, iz) + kernel[5](ix, iy, iz); | ||
| } |
Comment on lines
+78
to
+80
| const int n = static_cast<int>(coords_deg.size()); | ||
| if (n < 2) return _0_CR; | ||
|
|
- Modified the `dep_anom` function to build depth anomalies using symmetric half-sines, improving clarity and performance. - Updated error handling to ensure `nz` is greater than 0 and that `zgrids` is strictly increasing. - Removed dependency on the `minpack` library for fitting parameters, simplifying the implementation. - Adjusted the perturbation pattern generation to directly use the new `dep_anom` function. - Changed the perturbation parameters in the example script to maintain consistency with the updated depth anomaly calculations.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
close #28