-
Notifications
You must be signed in to change notification settings - Fork 66
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Feature/optimization symbolic expr degree #1889
Conversation
vincentchabannes
commented
Jun 30, 2022
•
edited
Loading
edited
- Have you checked to ensure there aren't other open Pull Requests for the same change?
- Have you added an explanation of what your changes do and why you'd like us to include them?
- Have you written new tests for your changes?
- Have you successfully run the Feel++ testsuite with your changes locally?
- Have you written Doxygen comments in your contribution ?
- Optimization of the computation total degree of a symbolic expr (polynomial). Previously the algo complexity was clearly very bad ( double loop + recursion non-terminal). Now it's just an iterative version which is faster. For example, this computation was done with EIM->beta() online computation and the perf is huge.
- Reduce some log verbosity
- maybe instead of this change, we can store the expr expanded by the lambda
…on_symbolic_expr_degree
@@ -1125,7 +1153,7 @@ private : | |||
if ( M_isNumericExpression ) | |||
return; | |||
|
|||
std::vector<std::pair<GiNaC::symbol,int>> symbTotalDegree; | |||
std::vector<std::pair<GiNaC::symbol,uint16_type>> symbTotalDegree; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
warning could you keep int ?
I am making in another branch the polynomial degree dynamic like in Eigen (compile time or dynamic)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
here is a discussion also on uint vs int http://eigen.tuxfamily.org/index.php?title=FAQ#Why_Eigen.27s_API_is_using_signed_integers_for_sizes.2C_indices.2C_etc..3F
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am not sure to understand why it's a problem because, here, it's a local variable used in the algorithm (and I need to check if value is invalid_v< uint16_type> or not. If really an issue, we can change to int and check non negative value.