search

XENON1T / pax

The XENON1T raw data processor [deprecated]
BSD 3-Clause "New" or "Revised" License
16 stars 17 forks source link

Allow passing nd array to InterpolatingMap #722

Closed zhut19 closed 5 years ago

zhut19 commented 5 years ago

Part of #672, then retracted in #676. Now, instead of np.vectorize, this pr uses python build-in map function. The change made here won't affect processing speed, but only to enable inputting something like ((x0, x1 ... xi), (y0, y1 ... yi), (z0, z1 ... zi)) and returns (v0, v1 ... vi)

Please read before creating a Pull Request

Before you start a pull request (PR), please note that pax is public. Only the technical details of the code can be mentioned directly on GitHub. If the analysis details are necessary for the PR, please make a wiki note and link it. The pull request will focus on the technical aspects rather than the motivation for the feature request.

JelleAalbers commented 5 years ago

Hi Tianyu, this looks fine.

However, if you want to achieve both this vector-call feature and a massive speed boost, have a look at the InterpolatingMap changed we made for strax and S2-only (same code). I think it's just a few changes to __call__ of InterpolateAndExtrapolate. However, I did not look at backwards compatibility; you'd probably have to insert a few if statements to continue supporting the scalar-call convention used in pax. (Or you could add the new logic as a third get_value_arrays method)

zhut19 commented 5 years ago

Hi Jelle, thanks for the suggestions. I totally missed that kdtree.query can take multiple points as input. I still left part of the if condition back in InterpolatingMap for different error catching, in addition to the if in __call__ of InterpolateAndExtrapolate. Which logically is a bit redundant but in this way less changes are made to original code.