Add from_data to healsparse map#173
Conversation
|
I agree that your code snippet is going to be slow! There are lots of internal tricks for partial maps that are done to avoid this, but they only work if you are fully aligned with a coverage pixel so haven't been exposed publicly. I'm happy to review the changes and maybe suggest alternatives but ... I know healsparse isn't black formatted like some people prefer, and I'm not completely opposed to that, but that (a) should be a separate PR, and (b) there needs to be a commit with the functionality that is being proposed here. Right now I am having trouble identifying the key code. |
|
Sorry the formatter runs automatically when I save files in my environment. I have just pushed a commit that removes the formatting. There is no change to any existing code. There is just a new convenience method (called |
|
Thank you so much for using healsparse and pointing out this significant performance problem! I did some profiling on the use case of "a large number of pixels need to be set at once" and I found the bottleneck was primarily where the code was using The next thing is that it may be that you know what you're doing and don't want to check for unique pixels. This can save a bit more time, and you can (on that branch) do something like: As for this PR I note that it doesn't quite work because (a) I'm not entirely sure if this is necessary; if you could take a look at the |
|
Hi @erykoff. Sorry for the exceedingly slow response here. I can confirm I see similar performance with your changes that I do with the workaround I currently have implemented. We will happily use it whenever you release a new version of healsparse. I have gone ahead and rebased this PR onto main and fixed a few issues. Personally I find these types of convenience functions very useful, but I leave it to you to decide if it makes sense for the library. I am happy to write a few tests if you do want to keep it, just let me know. |
Hi Healsparse team!
We recently ran into an issue when serving data to users with the HealSparse map in the OpenCosmo toolkit. We work with partial maps that are written to disk in a format that is compatible with our tools and allows users to perform additional querying etc. before requesting the actual map. This means we have to produce Healsparse maps on the fly when the user actually requests data.
In this context, have a
dataarray and apixelarray. Previously, this is the solution we were using:While concise, this is extremely slow for higher nsides. I believe this is because all the checks and logic that needs to be done, which assume there may already be data in the sparse array.
We wrote up a short little function to produce these maps which is significantly faster than our original approach. I didn't see any such function in the Healsparse library itself, so I figured I'd make this PR and see if you think its worth upstreaming.
I haven't written any tests yet. Wanted to see if this would be useful and make sure its in a format you're happy with first.
EDIT: I just realized my linter made a bunch of changes to the file for formatting reasons. I ran autopep8 and that seems to have made it pass the CI again.