Change namespacing of Lebedev routines #16
Conversation
wavefunction91
added
bug
susilehtola
force-pushed
the
lebedev_namespace
branch
from
009c1c1 to
34b6ad5
Compare
| using weight_container = typename base_type::weight_container; | ||
|
|
||
| if( npts == 6 ) | ||
| detail::copy_grid<lebedev_laikov_6<RealType>>( points, weights ); |
There was a problem hiding this comment.
These need to be detail::lebedev::copy_grid to fix the compilation, but I see that you've also restored to maybe make this more general? I'm pretty pragmatic in these situations, if you have an idea on how we can reduce code bloat by sharing routines between this and the Beylkin Grids, I'm all for it.
There was a problem hiding this comment.
I fixed the compile.
And no, I don't have an idea how to reduce code bloat for this. Due to the templating, it looks like there will be instantiations of the copy_grid routine for each of the grids. Would it not suffice to use assignments?
susilehtola
force-pushed
the
lebedev_namespace
branch
from
34b6ad5 to
73bdea2
Compare
wavefunction91
left a comment
wavefunction91
left a comment
There was a problem hiding this comment.
The more I think about this, the more I think what was previously there is fine. There's nothing lebedev specific about it - it simply stays that there are a set of static instances of grids (specified by a template parameter StaticGrid) with members points and weights and to copy those quantities into a passed set of containers. The function would be identical for the AH grids you've added in #9.
As for whether it should be std::copy or operator=, it's a matter of style, they're going to trigger identical behaviour - std::copy just allows the containers to be of different types.
If we want to make it less lebedev-y, we can move it to a separate file, but I see no reason to scope it to the detail::lebedev namespace (unless I'm missing something? Feel free to correct me if you still think there's a reason to scope it).
|
|
||
| using point_container = std::vector< point_type >; | ||
| using weight_container = std::vector< weight_type >; | ||
| } // namespace IntegratorXX |
There was a problem hiding this comment.
This is in the wrong location and is what is causing the current compile errors for the unit tests. It needs to be at the end of the file to properly scope LebedevLaikov
There was a problem hiding this comment.
^^ FWIW, I tested this locally and it works
|
Changes included in #9 |
2 participants
The Lebedev routine has an improperly namespaced copy routine. This PR puts that in the right namespace.
Alternatively, one could live with a single copy of this routine, but I don't understand the logic and layout of the code well enough to decide where it would go.