Add Nan/infinite guard to gauss_kronrod_quadrature (early exit) - #59
Merged
Merged
Conversation
- Do not include legend box if there is no legend (Axect#58) - Add rtol for Broyden method - High-level macros for root finding
Owner
|
Hi @GComitini, Thank you for your proposal to improve the performance of I agree that adding a check like Regarding the performance impact on well-behaved integrals, I don't expect it to be significant since the additional check is relatively cheap. I'll merge it now and release next version soon including this change. Thanks again for taking the time to analyze the issue and propose a solution! |
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.
Hello there! While I haven't had time yet to think about the complex Lagrange polynomials, I would like to propose a "performance improvement" for
gauss_kronrod_quadrature.Today my program got stuck while using the latter to do numerical integrations. After some digging I found out that
GandKingauss_kronrod_quadraturehad been equal toNaNfor a very large number of iterations and/or interval subdivisions. Ultimately, the reason why my program got stuck was issues of convergence in the integral itself. However, ifgauss_kronrod_quadraturehad detected those issues (to be specific: thoseNaNs) earlier, the program would have failed immediately instead of having to exhaust all iterations for all the interval subdivisions.Looking at the code,
if
Gis infinite orNan(in which case, for the record,(G - K).abs() < tol_currisfalse), and if we either reached the end of precision in the interval's subdivision (a == b), or we exhausted the maximum number of iterations for the given subinterval (max_iter == 0), thenGwill be added toIand the loop will go on. At this point, however,I, which is the final result, is already tainted: it will be either infinite orNaNand further operating on it during later iterations of the loop won't change this. Thus, if! G.is_finite()(that is, ifGis either infinite orNan) before addingGtoI, one could as well exit early with whatever meaningless result one has at that point. IfIis finite andGis not finite,I + G = G, so one may as wellreturn G;at that point.To be very clear, this change constitutes a performance improvement only for integrals whose computation fails, by making some of them fail faster. What needs to be evaluated is the performance impact on well-behaved integrals. If you have benchmarking code in place I will be happy to run it on this MR branch and see what happens.