Skip to content
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

Refactor function parser #13773

Merged
merged 9 commits into from May 27, 2022
Merged

Conversation

drwells
Copy link
Member

@drwells drwells commented May 21, 2022

Followup to #13745 - we can avoid duplicating a lot of code by adding another base class. We need to use a separate base class (instead of just functions) so that we can use the PIMPL idiom.

@drwells drwells force-pushed the refactor-function-parser branch 2 times, most recently from e53a933 to d10f56d Compare May 21, 2022 20:12
Copy link
Member

@kronbichler kronbichler left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice simplification.

Comment on lines +161 to +200
return {// functions predefined by muparser
"sin",
"cos",
"tan",
"asin",
"acos",
"atan",
"sinh",
"cosh",
"tanh",
"asinh",
"acosh",
"atanh",
"atan2",
"log2",
"log10",
"log",
"ln",
"exp",
"sqrt",
"sign",
"rint",
"abs",
"min",
"max",
"sum",
"avg",
// functions we define ourselves above
"if",
"int",
"ceil",
"cot",
"csc",
"floor",
"sec",
"pow",
"erf",
"erfc",
"rand",
"rand_seed"};
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this list be static and the function return a const & to it?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could do that, but we only call this when we set up the muParser objects so its not going to make a measurable performance difference. I would prefer not to add extra static objects unless we need to.

@drwells
Copy link
Member Author

drwells commented May 26, 2022

ping

@masterleinad masterleinad merged commit 87d6664 into dealii:master May 27, 2022
mkghadban pushed a commit to OpenFCST/dealii that referenced this pull request Sep 8, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants