-
Notifications
You must be signed in to change notification settings - Fork 251
Do some error checking to avoid accessing invalid vector elements. #6687
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
base: main
Are you sure you want to change the base?
Conversation
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.
Thank you for addressing this!
If the code crashes in the case of no plugin being selected, wouldn't it make sense to change the assert here to checking that the number of plugins is exactly 1 rather than <= 1
?
scratch.face_material_model_inputs, | ||
scratch.face_material_model_outputs); | ||
Assert (heat_flux.size() == n_face_q_points, | ||
ExcMessage("The boundary heat flux plugins have not returned information " |
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.
ExcMessage("The boundary heat flux plugins have not returned information " | |
ExcMessage("The boundary heat transfer coefficients have not returned information " |
I fixed the wording of the error message. As for this:
We can't. If you don't select heat transfer boundaries, you still read these parameters (even though we don't use them) and the default is a list of size zero. So that has to be allowed as part of regular operations -- it just needs to be a list of size one when these plugins are actually used. So my proposition would be to not touch these lines, leave the assertion in that we have here, and require that there is at least one plugin in the way discussed in #6686. Does that sound good? |
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.
So my proposition would be to not touch these lines, leave the assertion in that we have here, and require that there is at least one plugin in the way discussed in #6686. Does that sound good?
Yes, that makes sense to me!
scratch.face_material_model_outputs, | ||
scratch.face_finite_element_values->get_normal_vectors()); | ||
Assert (heat_flux.size() == n_face_q_points, | ||
ExcMessage("The boundary heat transfer coefficient plugins have not returned information " |
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 think you misunderstood my previous comment. This was the correct wording of the error message (we're checking heat flux)
ExcMessage("The boundary heat transfer coefficient plugins have not returned information " | |
ExcMessage("The boundary heat flux plugins have not returned information " |
Only the message below is for the heat transfer coefficient.
Assert (heat_flux.size() == n_face_q_points, | ||
ExcMessage("The boundary heat transfer coefficient plugins have not returned information " |
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.
This is correct as boundary heat transfer coefficient, but
Assert (heat_flux.size() == n_face_q_points, | |
ExcMessage("The boundary heat transfer coefficient plugins have not returned information " | |
Assert (heat_transfer_coefficients.size() == n_face_q_points, | |
ExcMessage("The boundary heat transfer coefficient plugins have not returned information " |
Good points, thanks for looking over it again! I fixed as suggested. |
Catch the error reported in https://community.geodynamics.org/t/error-when-robin-boundary-condition-is-applied/4302 at an earlier stage so we can give a useful error message. As shown there, right now we simply access an invalid array element.
Related to #6686