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

SideIntegralVariablePostprocessor doesn't throw an error if getting a material that doesn't exist #9835

Closed
cpgr opened this issue Sep 13, 2017 · 7 comments
Labels
C: Framework T: defect An anomaly, which is anything that deviates from expectations.

Comments

@cpgr
Copy link
Contributor

cpgr commented Sep 13, 2017

Description of the enhancement or error report

So, I just discovered that if a material property requested by a postprocessor based on SideIntegralVariablePostprocessor doesn't exist, the postprocessor will happily run producing junk output rather than throw an error.

Rationale for the enhancement or information for reproducing the error

This behaviour should produce an error

Identified impact

(i.e. Internal object changes, limited interface changes, public API change, or a list of specific applications impacted)
Make this postprocessor more robust

@permcody
Copy link
Member

Ping @idaholab/moose-team - I'm not sure if this is new, or existing but this is a problem. We need to replicate, create a test case and fix!

@permcody permcody added C: Framework P: critical T: defect An anomaly, which is anything that deviates from expectations. labels Sep 13, 2017
@lindsayad
Copy link
Member

Sounds like same kind of problem reported in #5309 for integrated boundary conditions and #9482 for dgkernels.

@aeslaughter
Copy link
Contributor

This might be related, but many years ago I worked on getting the boundary material properties checking to work: FEProblemBase.C:4694. I obviously didn't get it working, but this might be a starting point.

@andrsd
Copy link
Contributor

andrsd commented Sep 13, 2017

This check is more complicated to do, because BCs are set on BoundaryIDs, while materials are on a) blocks for the volumetric computation, b) blocks for side computation (used by BCs, for example), c) boundaries for boundary restricted materials.

Ideally, we would not have b), but the reason we have it is this. We have a subdomain 1, so people say put material A on block 1. Then they put a BC on a side of subdomain 1 and they expect their BC to pick the material props from block 1. We could remove this and tell people to add a special side restricted material. Or we could figure out what boundary IDs is block 1 connected to and add several instances of boundary restricted materials. That would remove b) and side checking would be based only on boundary IDs. But, I think there might be some small details that I do not see right now, deep inside other systems, that might actually rely on the fact that we have side material index by a subdomain ID. But if we can find the proper corresponding boundary ID to use instead, this change is possible.

@YaqiWang
Copy link
Contributor

It could increase the users' burden quite a bit if we decide to remove the other two copies of material for evaluating the properties on sides. The current design might not be a bad choice. Relevant codes for materials can be put in one source file. Although the check will be complicated, it should be doable.

@permcody
Copy link
Member

@andrsd hits it on the head. Doing coverage checks in MOOSE is getting increasingly difficult because of the number of combinations of suppliers and consumers and the various ways the can be restricted. This doesn't even address any temporal restrictions that are possible through the control system.

I envision a system that can perform all supply and demand checks that works by simply iterating over the mesh up front and making sure that everything demanded is supplied. This might be easier than the complication of trying to figure out boundaries attached to subdomains, etc. There are just so many edge cases that it's probably just easier to break it down element by element.

@YaqiWang
Copy link
Contributor

While then the check has to be done on every time step. Either way, the check needs to be done. Not against the up-front check idea though.

aeslaughter added a commit to aeslaughter/moose that referenced this issue Sep 13, 2017
aeslaughter added a commit to aeslaughter/moose that referenced this issue Sep 13, 2017
aeslaughter added a commit to aeslaughter/moose that referenced this issue Sep 13, 2017
aeslaughter added a commit to aeslaughter/moose that referenced this issue Sep 13, 2017
aeslaughter added a commit to aeslaughter/moose that referenced this issue Sep 13, 2017
aeslaughter added a commit to aeslaughter/moose that referenced this issue Sep 13, 2017
aeslaughter added a commit to aeslaughter/moose that referenced this issue Sep 14, 2017
aeslaughter added a commit to aeslaughter/moose that referenced this issue Sep 19, 2017
aeslaughter added a commit to aeslaughter/moose that referenced this issue Sep 19, 2017
jarons pushed a commit to jarons/moose that referenced this issue Oct 5, 2017
jarons pushed a commit to jarons/moose that referenced this issue Oct 5, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C: Framework T: defect An anomaly, which is anything that deviates from expectations.
Projects
None yet
Development

No branches or pull requests

6 participants