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

Adding parsing for double indexed/separated fields #6459

Merged
merged 1 commit into from Mar 9, 2016

Conversation

snschune
Copy link
Contributor

Concept of parsing double index objects. Half of it is missing still but I wanted to get feedback
before proceeding too far.

  • this will need libMesh::print_helper(std::ostream & os, const std::vector<vector<P> >* param)
  • should it support double indexed objects of type Point or RealVectorValue?
  • specialization of dynamicCastAndExtractDoubleIndex for MooseEnum
  • this will need a Moose unit test, but I am not sure how to set it up yet.

dynamicCastAndExtractDoubleIndex(PostprocessorName , it->second, full_name, it->first, in_global, global_params_block);
dynamicCastAndExtractDoubleIndex(VectorPostprocessorName, it->second, full_name, it->first, in_global, global_params_block);
dynamicCastAndExtractDoubleIndex(OutputName , it->second, full_name, it->first, in_global, global_params_block);
dynamicCastAndExtractDoubleIndex(MaterialPropertyName , it->second, full_name, it->first, in_global, global_params_block);
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 not sure I want to support every type in the double syntax. It's unlikely that we'll use very many of them. I'd suggest we add the types you want and leave the rest out until there's a use case and better yet, a test case to support them.

@permcody
Copy link
Member

permcody commented Mar 1, 2016

This is looking pretty good so far.

@snschune
Copy link
Contributor Author

snschune commented Mar 2, 2016

@permcody testing in regular tests or unit tests?

@permcody
Copy link
Member

permcody commented Mar 2, 2016

Either is fine. You might find it painful to build a Parser object in the Unit Tests... I'm not sure though maybe it's easy.

@snschune
Copy link
Contributor Author

snschune commented Mar 2, 2016

I have added a single test for reading double indexed Reals.I realized that there will be an issue
with VariableName. Coupleable assumes that these come as vectors and hence coupledValue takes only one index. To enable double indexed coupled variables would probably require a lot of work, so I'd just disable this capability for now.
Opinions?
Do you need to test the Parser functions for all permissible data types?

@snschune snschune changed the title [WIP] Adding parsing for double indexed/separated fields Adding parsing for double indexed/separated fields Mar 2, 2016
@permcody
Copy link
Member

permcody commented Mar 2, 2016

Well that would be nice, but I still think we should pare down the list of permissible types. Do we really think we are going to need a double indexed list of function_names for instance? Let's just add the types we need now. If more needs arise later, we'll add PRs with tests later.

@snschune
Copy link
Contributor Author

snschune commented Mar 3, 2016

FunctionName and primitive types are actually the ones that we want in Rattlesnake. I'd definitely remove the following:

FileName, FileNameNoExtension, MeshFileName, MultiAppName,

but I could live with the unmentioned ones going as well. Once we trimmed this, I'll add tests
for all supported types.

@friedmud
Copy link
Contributor

friedmud commented Mar 3, 2016

I actually vote for adding all the ones we can think of / all the ones
Sebastian cares to do right now. As long as they are tested, why not?

This is going to be a new "first class", supported input syntax: so let's
make it work like people expect...
On Wed, Mar 2, 2016 at 7:10 PM Sebastian Schunert notifications@github.com
wrote:

FunctionName and primitive types are actually one that we want in
Rattlesnake. I'd definitely remove the following:

FileName, FileNameNoExtension, MeshFileName, MultiAppName,

but I could live with the unmentioned ones going as well. Once we trimmed
this, I'll add tests
for all supported types.


Reply to this email directly or view it on GitHub
#6459 (comment).

@snschune snschune force-pushed the double_indexed_parse_6442 branch 2 times, most recently from 25d6d79 to 6c9248b Compare March 3, 2016 22:05
@snschune
Copy link
Contributor Author

snschune commented Mar 3, 2016

This needs libmesh libMesh/libmesh#846
Is this going to be in the next libmesh update?

@permcody
Copy link
Member

permcody commented Mar 3, 2016

Yes, it'll be in the next one.

On Thu, Mar 3, 2016 at 3:49 PM Sebastian Schunert notifications@github.com
wrote:

This needs libmesh libMesh/libmesh#846
libMesh/libmesh#846
Is this going to be in the next libmesh update?


Reply to this email directly or view it on GitHub
#6459 (comment).

@snschune snschune force-pushed the double_indexed_parse_6442 branch 2 times, most recently from e726c4c to 8edd2bd Compare March 9, 2016 16:25
@permcody
Copy link
Member

permcody commented Mar 9, 2016

We should probably do this as a unit test at some point so we can test the succeeds and fails equally well. I won't make you do that for this test though. This will get us pretty far down the path towards doing that.

@snschune
Copy link
Contributor Author

snschune commented Mar 9, 2016

I am done with this for now and ready to take the heat.

@permcody
Copy link
Member

permcody commented Mar 9, 2016

👍

permcody added a commit that referenced this pull request Mar 9, 2016
Adding parsing for double indexed/separated fields
@permcody permcody merged commit 541aa0a into idaholab:devel Mar 9, 2016
@YaqiWang
Copy link
Contributor

YaqiWang commented Mar 9, 2016

I will suggest to create another PR for using this new capability in

https://github.com/idaholab/moose/blob/devel/framework/src/preconditioners/SingleMatrixPreconditioner.C

for the coupled_groups parameter.

That can clean the code up quite a bit.

@snschune
Copy link
Contributor Author

snschune commented Mar 9, 2016

Yes, why not deploy it.

2016-03-09 14:00 GMT-07:00 Yaqi notifications@github.com:

I will suggest to create another PR for using this new capability in

https://github.com/idaholab/moose/blob/devel/framework/src/preconditioners/SingleMatrixPreconditioner.C

for the coupled_groups parameter.

That can clean the code up quite a bit.


Reply to this email directly or view it on GitHub
#6459 (comment).

@snschune
Copy link
Contributor Author

snschune commented Mar 9, 2016

What uses SingleMatrixPreconditioner?
Note: we excluded NonlinearVariableName as type for double indexed parsing
because it is inconsistent with Coupleable which is made for a vector.

@dschwen
Copy link
Member

dschwen commented Mar 9, 2016

It is used in a bunch of phase field examples and tests.

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

5 participants