Feature/integration part6 #83
Conversation
| @@ -11,6 +11,8 @@ FetchContent_Declare( eigen | |||
| GIT_REPOSITORY https://gitlab.com/libeigen/eigen.git | |||
| GIT_TAG d0bfdc1658ca0b4c659fd3702c351d2c2cdc876c # 3.4.1 branch on July 26, 2023 | |||
| ) | |||
| # the next line prevents eigen from populating the .cmake/packages folder | |||
There was a problem hiding this comment.
correct me if I'm wrong, but I think this shoul prevent population of ~/.cmake/packages, yes?
There was a problem hiding this comment.
indeed. .cmake is in your user directory.
| return static_cast< const Derived* >( this )->integrate( xLeft, xRight, yLeft, yRight ); | ||
| if ( xLeft == xRight ) { | ||
|
|
||
| return I( 0. ); |
There was a problem hiding this comment.
What is the reason for this check? xRight - xLeft if xRight and xLeft are equal should be 0 (even with float point stuff). For speedup, this only makes sense to me if we expect a call where xLeft and xRight are equal a lot of times - then we can bypass the other calculations. But is this what we expect? I would have thought overwhelmingly this should not be the case. In which case we are just adding a needless check to every integration, yes?
I guess for integrations where no term (xRight - xLeft) exists in the return one might do this check due to float point stuff possibly throwing you off
There was a problem hiding this comment.
Some of the data do have jumps in the x grid (so duplicate x values) but they are rare, and maybe occur once or twice if they do occur in grids of over 1000 points. As you saw, I originally wrote the test in the individual integrators. Since it is the same code in all of them, I moved it to the base class.
We do need to guard against division by zero (either xRight - xLeft or std::log(xRight/xLeft)) - which can happen in all but the lin-lin integrator, lin-log integrator and the first moment histogram integrator. Still, we might be able to do that at a different level (i.e. where the integration is used).
This actually gives me an idea. We already subdivide the x,y data in regions based on interpolation type and the presence of jumps (to ensure each region only has unique x values). So we can perform the integration over those and thus ensure no need for testing equal x values.
|
Closing this PR as we move to internal gitlab. |
This updates some of the integration components in scion: