rlancaste / stellarsolver

GNU General Public License v3.0
89 stars 47 forks source link

Get rid of more astrometry.net dependencies #72

Closed mgreter closed 3 years ago

mgreter commented 3 years ago

Hi folks

This is not really a bug report, but more of a heads up :) I actually did something very similar as you do here at: https://github.com/mgreter/astrometrylib

I found your project a bit late in my attempt and since I IMHO got even a little bit further at stripping down than you did here, I wanted to share some of my approaches. Maybe you would like to take a peek at how I did the port 😉

In the end I only have 65 files from astrometry.net left in my tree (stellarsolver around 400?). I also got completely rid of an-qfits and the bl.h lists. The former was of course the hardest, since that meant I would need a different loader for the indexes. Instead of reinventing that wheel in C again, I decided it would be easier to just convert the indexes to a more (fixed) binary format. This has of course the drawback that the original indexes are not directly compatible, but this really got rid of a lot of the complicated dependencies. And the new binary index loader is only ~250 lines and it IMO really makes sense to have them very easily readable for this purpose.

I also added multi-threading to my POC in order to check that possibility. Other than that pretty much everything else is hardcoded in my POC yet, although at the right places to implement something useful instead.

Hope you don't mind me opening this github issue.

Looking forward to your feedback!

rlancaste commented 3 years ago

Thank you very much for reaching out. Collaboration can really help us make all these things better.

One thing that I worry about is if we get too far away from the original astrometry.net code, then if we want to include revisions they make in their code in the future, that might be much more difficult. However, at this point, I think with all the changes I had to make, it might already be too much for that.

Another thing is that I don't want to get too far away from the astrometry index files, since that allows compatibility between the different plate solvers. One thing I was really trying to do with StellarSolver is allow it to use the internal solver as well as different external ones in exactly the same way, using the same index files, so that lots of testing can be done, and the user who is using StellarSolver can choose to use the internal solver, or to use one of the external or online options if there is an issue with my code. It is a library that works with many different solvers, not just the internal one.

I will definitely check out what you have done when I get a chance, but recently I have had a lot more work to do in my real job, so that has cut down on the amount of time I have had for this. But yeah, I will check it out.

Also, an interesting update is that StellarSolver has been incorporated into KStars and is now solving real astronomical images on many systems around the world! I am now using their feedback and results to further make improvements to both KStars and StellarSolver.

mgreter commented 3 years ago

Thanks for your response!

Absolutely understand the points you're making. I think it should still be possible to incorporate improvements from upstream though, I haven't really changed much of the code, just cut out a lot. And the alternative index format is rather simple to create from the original fits files. But I get the point. IMO it was worth it since it got rid of so many dependencies I don't really need.

rlancaste commented 3 years ago

Yeah, I changed so much to get it working as well on different operating systems like Windows and MacOS as it does on Linux, and made so many changes to correct memory leaks and crashes that the original astrometry.net left open, that I'm not sure how I could implement changes from upstream. . . but I did try to leave the same comment every time I made a change, so maybe I could search for the spots changes need to be made and make them again for a new version. . .