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 parsed material backend and add ADParsedMaterial #15331

Merged
merged 10 commits into from
May 28, 2020

Conversation

dschwen
Copy link
Member

@dschwen dschwen commented May 21, 2020

This mostly lays the groundwork for dual number support in Parsed and Derivative parsed Materials. This includes a libmesh update to allow for JIT compilation with dual numbers.

Refs #15207

@moosebuild
Copy link
Contributor

Job Precheck on a89efcc wanted to post the following:

Your code requires style changes.

A patch was auto generated and copied here
You can directly apply the patch by running, in the top level of your repository:

curl -s https://mooseframework.inl.gov/docs/PRs/15331/style.patch | git apply -v

Alternatively, with your repository up to date and in the top level of your repository:

git clang-format bb526784f0d3d430f65655b691036d0e679d75e1

@@ -1 +1 @@
Subproject commit 30d38fc9b5bc608655d033117a8d5cb59c57ae4c
Subproject commit cd8b4510adc28c1b480dda33cb7f449831f2fad1
Copy link
Contributor

Choose a reason for hiding this comment

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

Caution! This contains a submodule update

@@ -1 +1 @@
Subproject commit 30d38fc9b5bc608655d033117a8d5cb59c57ae4c
Subproject commit cd8b4510adc28c1b480dda33cb7f449831f2fad1
Copy link
Contributor

Choose a reason for hiding this comment

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

Caution! This contains a submodule update

@dschwen dschwen force-pushed the aderivativeparsed_15207 branch 2 times, most recently from 2e8cb8c to 68d7bc4 Compare May 21, 2020 22:45
@@ -1 +1 @@
Subproject commit 30d38fc9b5bc608655d033117a8d5cb59c57ae4c
Subproject commit cd8b4510adc28c1b480dda33cb7f449831f2fad1
Copy link
Contributor

Choose a reason for hiding this comment

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

Caution! This contains a submodule update

@@ -1 +1 @@
Subproject commit 30d38fc9b5bc608655d033117a8d5cb59c57ae4c
Subproject commit cd8b4510adc28c1b480dda33cb7f449831f2fad1
Copy link
Contributor

Choose a reason for hiding this comment

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

Caution! This contains a submodule update

@@ -1 +1 @@
Subproject commit 30d38fc9b5bc608655d033117a8d5cb59c57ae4c
Subproject commit cd8b4510adc28c1b480dda33cb7f449831f2fad1
Copy link
Contributor

Choose a reason for hiding this comment

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

Caution! This contains a submodule update

@@ -1 +1 @@
Subproject commit 30d38fc9b5bc608655d033117a8d5cb59c57ae4c
Subproject commit cd8b4510adc28c1b480dda33cb7f449831f2fad1
Copy link
Contributor

Choose a reason for hiding this comment

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

Caution! This contains a submodule update

@@ -1 +1 @@
Subproject commit 30d38fc9b5bc608655d033117a8d5cb59c57ae4c
Subproject commit cd8b4510adc28c1b480dda33cb7f449831f2fad1
Copy link
Contributor

Choose a reason for hiding this comment

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

Caution! This contains a submodule update

@dschwen
Copy link
Member Author

dschwen commented May 23, 2020

Why is a PR that contains a libmesh update tested against a two week old conda libmesh package?! What is the workflow supposed to be now?

@dschwen
Copy link
Member Author

dschwen commented May 23, 2020

External App tests (Conda libMesh) is a misleading name, as it builds the libmesh submodule and does not use the conda package.

@dschwen dschwen requested a review from lindsayad May 26, 2020 18:15
@lindsayad
Copy link
Member

lindsayad commented May 26, 2020

You will want to change "version" in conda/libmesh/meta.yaml, a la here

You need to do some change in the conda/libmesh directory (a change observable through git diff) in order to trigger a new conda build. Editing the version is a sufficient change

@dschwen
Copy link
Member Author

dschwen commented May 26, 2020

You need to do some change in the conda/libmesh directory (a change observable through git diff) in order to trigger a new conda build. Editing the version is a sufficient change

Ok, cool. Apparently I didn't pay attention in class when the new workflow was introduced...

@@ -1 +1 @@
Subproject commit 30d38fc9b5bc608655d033117a8d5cb59c57ae4c
Subproject commit cd8b4510adc28c1b480dda33cb7f449831f2fad1
Copy link
Contributor

Choose a reason for hiding this comment

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

Caution! This contains a submodule update

Copy link
Member

@lindsayad lindsayad left a comment

Choose a reason for hiding this comment

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

I think this looks really good @dschwen

using FunctionParserUtils<T>::_eval_error_msg; \
using FunctionParserUtils<T>::_func_params

// Helper class to pic the correct function parser
Copy link
Member

Choose a reason for hiding this comment

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

particle in cell?

// fill the parameter vector, apply tolerances
for (unsigned int i = 0; i < _nargs; ++i)
{
if (_tol[i] < 0.0)
_func_params[i] = (*_args[i])[_qp];
else
{
a = (*_args[i])[_qp];
auto a = (*_args[i])[_qp];
Copy link
Member

Choose a reason for hiding this comment

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

I'm always curious whether to optimize for the probably much more popular use case which is builtins vs. optimize for the potentially much more expensive but maybe much less used user-defined type. I'll let you decide whether you want to copy here or catch as const auto &

Copy link
Member Author

Choose a reason for hiding this comment

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

Since FParser doesn't know MooseArrays I have to prepare a separate parameter vector. Good point about the copying though. I could add a separate mode in which parameters are passed as a vector of pointers rather than a vector of copied values. I would prefer to leave that for the next steps though.

@dschwen
Copy link
Member Author

dschwen commented May 27, 2020

Ah, I think now I'm seeing errors due to Gary's recent PR. I think we made some similar colliding changes.

@hugary1995
Copy link
Contributor

I came in to say thanks for this long-awaited PR until I saw my name.

@dschwen
Copy link
Member Author

dschwen commented May 27, 2020

Ha, no worries. It is up to me to rebase and fix this!

@@ -1 +1 @@
Subproject commit 30d38fc9b5bc608655d033117a8d5cb59c57ae4c
Subproject commit cd8b4510adc28c1b480dda33cb7f449831f2fad1
Copy link
Contributor

Choose a reason for hiding this comment

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

Caution! This contains a submodule update

@moosebuild
Copy link
Contributor

Job Mac Test on 9f2d896 : invalidated by @milljm

Deleted stale environment

@dschwen
Copy link
Member Author

dschwen commented May 28, 2020

I have a marmot PR ready

@@ -1 +1 @@
Subproject commit 7b841db905481fcdd53fa823184861fa1061960f
Subproject commit f7c572fcde16c25fff3bc2fc69a4f92f555bd945
Copy link
Contributor

Choose a reason for hiding this comment

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

Caution! This contains a submodule update

@moosebuild
Copy link
Contributor

Job Documentation on 9703f7e wanted to post the following:

View the site here

This comment will be updated on new commits.

@dschwen
Copy link
Member Author

dschwen commented May 28, 2020

@lindsayad the ZapdosCrane fix should be easy as well. ADFunctionPtr is now called SymFunctionPtr, and FunctionParserUtils is now an is_ad template, so make that FunctionParserUtils<false>.

@dschwen
Copy link
Member Author

dschwen commented May 28, 2020

Oh, ZapdosCrane is public. I guess I'll just make a PR myself then!

@dschwen
Copy link
Member Author

dschwen commented May 28, 2020

Copy link
Member

@lindsayad lindsayad left a comment

Choose a reason for hiding this comment

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

Very exciting stuff

auto result =
JITCompileHelper("DualReal",
ADFPARSER_INCLUDES,
"#include \"DualReal.h\"\n#include \"metaphysicl/dualnumberarray.h\"\n");
Copy link
Member

Choose a reason for hiding this comment

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

@dschwen I should have caught this on review. Just #include ADReal.h and that should get you everything you need.

Copy link
Member Author

Choose a reason for hiding this comment

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

ah, cool, I'll make a fixup PR

dschwen added a commit to dschwen/moose that referenced this pull request May 29, 2020
dschwen added a commit to dschwen/moose that referenced this pull request May 29, 2020
dschwen added a commit to dschwen/moose that referenced this pull request May 29, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants