-
Notifications
You must be signed in to change notification settings - Fork 147
Add continuum algorithms #106
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
Conversation
|
Hi! This is implementation I did for learning purposes, and then decided to contribute here, if you're iterested. To remove continuum from full cuprite image, on my high end computer, takes about 70 seconds. Equivalent implementation in C, takes under 1 second. I saw there is library called hsdar in R language, that has its own implementation, maybe I could try to compare them, if you'd like. Worst case scenario, it's O(N^2), where N is number of bands, but that is almost impossible. Other then that, I tried to squeeze as much performance as I could get. I will add doc strings to this pull request. Just waiting for initial comments. Also, I have plans to add support for, so called, segmented upper hull. But that would be another pull request. Thank you. |
|
Hi, Thank you for the PR! I'll try to take a closer look at this in the next few days but from a code perspective, it looks very well done and it would be great to have a continuum removal function in the package. Please do continue with your work on the PR. As for speed, a slow implementation is better than no implementation. We can try to figure out how to speed it up, if not through native python code, then perhaps via cython. Could you possibly provide an example plot or two of a spectrum, along with its continuum and with continuum removed? That would be really helpful in understanding how it is working. Thanks |
|
Adding segmented upper hulls could be good next move, to avoid long lines like on first few plots. Also there is function |
|
I've made some changes and added doc strings. |
9caa6d4 to
bb4dc53
Compare
|
Thanks. Do you have an approach for how you would handle a segmented upper hull? I think that would greatly improve the utility of the continuum removal for evaluating absorption bands. |
|
I took a look at hsdar implementation in R. I know a little bit of R, but main part is written in Fortan (for performance probably). I don't think it is to complicated to understand it. I already have some general idea of how it works. |
|
And, yes, since it is quasi convexity, all points of convex hull should belong to segmented hull too. So I would use current algorithm, and then break detected concave part even futher if possible. |
|
I will add it to this pull request, if you like it. Just need to update documentation, write tests, and do some performance evaluation. |
|
Yes, please do add this. I assume it significantly to processing time but I believe the increase in quality is worth it. |
|
I'm done. Ready for comments (or merge). |
|
I did profiling, there is nothing in the new function that creates bottleneck. That is what I wanted to check. All contributions to processing time is from creating convex hull between these new maxima. |
|
I'll go ahead an merge shortly. Can you provide an example of how one would typically use the functions? That will be helpful for when I update the website documentation. Also, if one wanted to get the continuum, as well as the continuum-removed spectrum, would there be any performance benefit to doing both operations in a single function? |
|
I made single function for convenience. Since continuum removal is just division by continuum, one can compute continuum, and then divide by it. That also requires extra memory if one does not need continuum alone. So I tought it might be good trade off to make it the way I did. As for the examples, I'll take a look now at other examples on the site and come up with something in line with others examples. |










Thought it might be good idea to add continuum computing and removing algorithms to make this library even more awesome and complete.