-
Notifications
You must be signed in to change notification settings - Fork 92
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
Use separate functions for creating quadrature rule instead of the QuadratureRule
constructor
#1001
Comments
I quite like the current symmetry with interpolation constructors. |
Yes it looks nice but for the interpolation there are not really multiple "choices". To me, this is a bit similar to how there exists a |
There should imo at least be a function that works for any reference shape and returns a good default |
I think it makes sense to factor out the functions to create the quadrature points and weights (especially to allow easy reuse for custom quadrature rules) that the constructor calls. But I also think it is nice to have the current constructor since it becomes very clear what type you create. I would find it more logical to provide the rule as a kwarg, but perhaps not enough to motivate another breaking change. |
Something like this? create_quadrature(::RefShape, order) -> some default rule
create_quadrature(Legendre(), ::RefShape, order)
create_quadrature(Lobatto(), ::RefShape, order)
... but if you extend this you probably just supply points and weights directly instead of building into this interface? |
Sorry, was on the phone writing yesterday... create_quadrature(type::Legendre, ::RefShape, order::Int) -> (weights, points)
create_quadrature(type::Lobatto, ::RefShape, order::Int) -> (weights, points)
function QuadratureRule{RefShape}(order::Int) where RefShape
weights, points = create_quadrature(Legendre(), RefShape, order::Int)
return QuadratureRule{RefShape}(weights, points)
end
function QuadratureRule{RefShape}(type, order::Int) where RefShape
weights, points = create_quadrature(type, RefShape, order::Int)
return QuadratureRule{RefShape}(weights, points)
end While I see @KristofferC's argument for |
I do not like this exact design, because it makes the changing the rule slightly harder on user side. So, for the user-facing part I suggest that we either keep the current design or follow something like Fredrik wrote down:
or create_quadrature(::RefShape, order) -> some default rule
create_quadrature(:legendre, ::RefShape, order)
create_quadrature(:lobatto, ::RefShape, order)
... Another argument which I have against writing down the exact name is that most users are not familiar with details on quadrature rules, at least from my experience. So having a choice beyond the default rule might cause more confusion than good. (Also, I am pretty sure that our Tet quad rule is not Gauss-Legendre type :)) |
We can just keep the old constructors around to make this non breaking if we come up with something in the future. |
#991 (comment)
I think the current way of creating quadrature rules is not ideal. For example we write:
but there are many quadrature rules and it isn't clear which one this creates etc. This was changed by me in #58 and 8 years later I am suggesting we change it back to standard functions like:
etc. Should be decided before 1.0 I guess.
The text was updated successfully, but these errors were encountered: