Use more descriptive assertions

[ajaust]

Hi,

I think it would make sense to have more descriptive assertions, especially with the changes in the new Python bindings (python_future). A lot of things changed in the API making it likely that things break in user code. I was a bit surprised that the new bindings not only change the number of arguments a preCICE function takes (set_mesh_vertices takes 3 arguments instead of 4 in the C++ interface), but also it expects different argument types/shapes.

In the old bindings set_mesh_vertices would accept a list of coordinates where it was basically a 1D list/array of length numPoints*dimension. The data passed to tthe function did not have to be an numpy array. In precice_future it worked for me only when I use 2D numpy.array with numPoints entries that have the coordinates (dimension).

I started with using an older code with the new interface that used a 1D list (no numpy) of coordinates. This was caught by the exception in line 135, but the exception was not really helpful as there was no explanation what I did wrong. From the documentation it is also not obvious what type/dimension the input arguments should have.

I would suggest to replace the line

assert(dimensions == self.get_dimensions())

by something like

assert(dimensions == self.get_dimensions(),"Descriptive message what failed and why (what did the binding expect here?)")

This holds true for the other assertions as well. At least for me as a user it was not immediately obvious why it failed and what the binding actually expects. This is also somehow related to the documentation of the bindings as discussed in precice/precice#450 and #3.

Thank you!

[IshaanDesai]

Thank you @ajaust for raising this important issue! I have added error message to the assertions in the new python_bindings here: precice/precice#547

[ajaust]

Great! Thank you. Sorry for opening another issue. I guess I only searched for "Python" and not for "Pythonbindings" in the issues before opening a new one.

[BenjaminRodenberg]

Great! Thank you. Sorry for opening another issue. I guess I only searched for "Python" and not for "Pythonbindings" in the issues before opening a new one.

No need for apologies. You issue actually triggered this PR.

[ajaust]

Ah. I missed that. Then I am happy with opening the issue. ;)

Thanks for the quick fix.

[IshaanDesai]

@ajaust you can take a look at precice/precice#547 and give further inputs on how you as a user of the python_bindings would like the errors to be structured :) I will incorporate the changes before merging!

[BenjaminRodenberg]

Some context on

In the old bindings set_mesh_vertices would accept a list of coordinates where it was basically a 1D list/array of length numPoints*dimension. The data passed to tthe function did not have to be an numpy array. In precice_future it worked for me only when I use 2D numpy.array with numPoints entries that have the coordinates (dimension).

In precice/precice#458 there is some discussion on the general question how to format vector valued data in preCICE. In precice/precice#496, I finally implemented the new interface that requires n x d array. Since 1D arrays of length n*d are not supported anymore, this change is API breaking, if you compare it to the old python bindings. Additonally, it is also not consistent with the C++ preCICE API, where we are restricted to 1D arrays, where vector data is flattened correspondingly, due to the pointer based API.

Even though, there are some drawbacks, we decided to do this step, since it allows us to consequently support numpy arrays with all their benefits.