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

Robustness problem with RemotePointEvaluation #13501

Closed
nfehn opened this issue Mar 7, 2022 · 3 comments · Fixed by #13547
Closed

Robustness problem with RemotePointEvaluation #13501

nfehn opened this issue Mar 7, 2022 · 3 comments · Fixed by #13547

Comments

@nfehn
Copy link
Contributor

nfehn commented Mar 7, 2022

What happens if marked_vertices passed to the constructor of RemotePointEvaluation contains false only?

I would expect that the search is just less efficient. However, I observed that all data arrays are filled with zeros. So I assume that no points have been found? I did not receive an error/exception. This is quite dangerous since the computed results are just incorrect without an error message.

In my opinion, this is a robustness problem in the current implementation. One typically starts with a vector of false and then marks relevant vertices. Assume this code does not work as expected. Then, the whole remote point evaluation does not work. This should not occur in my opinion since the part of code dealing with marked vertices is only there for performance reasons?

Is it correct that RemotePointEvaluation does not give error messages in case points are not found? If so, why is it implemented like this? Could one ask RemotePointEvaluation via a public member function whether the setup has been successful, e.g. how many points have not been found, so that user code can decide what to do with this information (assert, message to std::cout, etc.)?

@drwells
Copy link
Member

drwells commented Mar 9, 2022

We definitely need to document that function argument better. The description isn't enough to answer your question, and its not at all clear from the single sentence what would happen if everything were marked as false.

Is it correct that RemotePointEvaluation does not give error messages in case points are not found?

That behavior seems wrong, so I think everyone would agree the answer is 'no, things should not fail silently' in debug mode.

@peterrum
Copy link
Member

What happens if marked_vertices passed to the constructor of RemotePointEvaluation contains false only?
I would expect that the search is just less efficient. However, I observed that all data arrays are filled with zeros. So I assume that no points have been found? I did not receive an error/exception. This is quite dangerous since the computed results are just incorrect without an error message.
In my opinion, this is a robustness problem in the current implementation. One typically starts with a vector of false and then marks relevant vertices. Assume this code does not work as expected. Then, the whole remote point evaluation does not work. This should not occur in my opinion since the part of code dealing with marked vertices is only there for performance reasons?

This is not an issue of RPE. RPE just passes the vector further. If the user passes in a vector full of falses, we need to assume that it was intentionally (for the current process).

Is it correct that RemotePointEvaluation does not give error messages in case points are not found?

Yes.

If so, why is it implemented like this?

Because there application cases where it is not critical if a point has been found.

Could one ask RemotePointEvaluation via a public member function whether the setup has been successful, e.g. how many points have not been found, so that user code can decide what to do with this information (assert, message to std::cout, etc.)?

You could already force RPE to have a "one-to-one relation". In that case, there would be an assert thrown if it was successful or not. However, in the general case (DG) there is "one-to-one relation". In that case one could have got the information via get_point_ptrs() and check if the entries differ by at least 1. I have added a utility function in #13547 for that purpose.

@nfehn
Copy link
Contributor Author

nfehn commented Mar 17, 2022

This is not an issue of RPE. RPE just passes the vector further. If the user passes in a vector full of falses, we need to assume that it was intentionally (for the current process).

as @drwells said, this should at least be clearly documented.

Because there application cases where it is not critical if a point has been found.

This is not satisfactory in my opinion. I would argue that there are applications where it matters, see e.g. exadg/exadg#205 ... an application we already have developed before RPE came into dealii ... What brings you to the assumption that "0" would be the correct "neutral element". There are applications conceivable where "1" might be the correct "neutral element". -> The user must have the information whether points are found or not! From a general software design perspective, my answer would be instead: If a particular application does not need this information, it does not have to use it.

However, in the general case (DG) there is "one-to-one relation".

I assume you mean no "one-to-one relation". get_point_ptrs() sounds unclear, I also don't know what "if the entries differ by at least 1" means? I am looking forward to study/use #13547!

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 a pull request may close this issue.

3 participants