corrected typos and added missing description of GenericQuadrature #764
Conversation
| @@ -99,7 +100,7 @@ get_weights(q::GenericQuadrature) = q.weights | |||
| get_name(q::GenericQuadrature) = q.name | |||
|
|
|||
| function GenericQuadrature(a::Quadrature) | |||
| GenericQuadrature(get_coordinates(q),get_weights(a),get_name(q)) | |||
| GenericQuadrature(get_coordinates(a),get_weights(a),get_name(a)) | |||
There was a problem hiding this comment.
Hi @kishore-nori. Thanks for your changes. How can it be that this was working even with these typos? I guess that this particular part of the code was not being tested, agreed? Perhaps you may also add a test to the test suite at the appropiate place to test this particular scenario.
There was a problem hiding this comment.
Yes @amartinhuertas, I was surprised too, I think the code wasn't specialising to this method, since only standard Quadrature was being used and it might have never happened so that a Quadrature which is not GenericQuadrature was passed through it in the tests or examples till now. Yes.. I ll think of a test for testing this in the ReferenceFEsTests/QuadratureTests.jl.
There was a problem hiding this comment.
Quadrature has only one concrete subtype which is the GenericQuadrature and Quadrature is an abstract type. I am not exactly sure how to come up with a test for it. Currently, if evaluated it using get_coordinates, get_weights and get_name it would result in @abstractmethod call as a fall back.
If I add a new concrete subtype DummyQuadrature similar to GenericQuadrature, and run test_quadrature it would result in the @abstractmethod call resulting in an ERROR: This function belongs to an interface definition and cannot be used. which although verifies the working of the function and if free from typos specific errors but results in an error where the test wouldn't pass.
Does Gridap follow a generic idea for testing abstract types? Would be instructive for me to have such example..
There was a problem hiding this comment.
ok. I see what you mean. We can accept the PR like now. But keep in mind that we may add a test once we have more subtypes of GenericQuadrature as far as these do not overload the GenericQuadrature constructor with an specialized version.
No description provided.