Jusas / WatneyAstrometry

Astrometric solver library and app for .NET Core
Apache License 2.0
31 stars 0 forks source link

Not really an issue, collaboration #2

Closed rlancaste closed 2 years ago

rlancaste commented 2 years ago

Hi There, I am Rob Lancaster and I have been involved with KStars and INDI since about 2016. For years, we used astrometry.net with KStars and added ASTAP support, but we always ran into issues as you found. We always had huge issues with configuration files, users having multiple python versions, misconfigurations etc. And of course astrometry.net doesn't natively run on windows and needs a compatibility layer So in 2020, around the time of the start of the pandemic, I developed StellarSolver, a cross platform Library that a cross platform Qt program can use to plate solve in a variety of ways. As a part of StellarSolver, I did dive into the astrometry.net code and made it work on Windows natively along with making it work as an internal library needing no external configuration files. So it does have a built-in stripped down version of astrometry.net in there. But in addition to this internal solver, it can also use ASTAP, local astrometry.net, Windows ANSVR, Windows Cygwin, and online astrometry.net. Its real strength however is in how it can use most of these solving methods in a parallelized manner and can thus blind solve many jpegs with them in less than 10 seconds ( or less than 1 second on my 16 core I9 processor). The whole project with all solving methods works equally well on Windows, Mac, and Linux. I also have had many conversations with Han who created ASTAP.

Your solver sounds very interesting and I would be interested in supporting that one as well if you think it is ready and able to work on all platforms. I will download it and give it a shot.

rlancaste commented 2 years ago

Here is my StellarSolver Repository: https://github.com/rlancaste/stellarsolver

I am getting ready to do a really big release, version 2.0. If your solver is ready, I can probably include that before releasing.

Jusas commented 2 years ago

Hi Rob!

This is really interesting - I was just in fact looking into how the Watney could be enabled for KStars and found out that its code base for alignment has indeed changed a lot and uses the StellarSolver now. Our interests are definitely aligned! Watney is on its early stages in regards to software support and it would be really nice if you are indeed interested in adding support for it. You have my full attention, anything you need, I'm available.

Watney is of course very fresh and with that there may be minor interface changes but my intent has always been to maintain good backwards compatibility. The API app has versioning applied to it, meaning that if the interface needs changes then a new version will be published with the old version still co-existing alongside (old one will be marked as deprecated, and then maybe removed in the next big release).

So yes, I'd be happy to collaborate!

Jusas commented 2 years ago

Oh by the way, forgot to mention that I've already run Watney via its astrometry.net compatibility API with StellarSolver in KStars. Works as expected. That's how I've been testing it in action.

Could be useful to build support for both the CLI solver and the API solver - however I'd prioritize the CLI solver if you need to choose, since the compatibility API already enables it use even if not with optimal configuration.

rlancaste commented 2 years ago

Yes I agree with your last point, because I actually was originally going to provide support for running windows ANSVR as a separate option like the online astrometry.net. But then I found that in fact, I was able to directly access the astrometry.net within the ANSVR folder and treat it EXACTLY like we do with the local astrometry.net on MacOS and Linux, which was really really cool and in fact it ran much much faster than accessing it using the traditional ANSVR access API. So I realized that if somebody was running the ANSVR version of astrometry.net on the same computer as KStars it made NO sense to use the "online" method at all. The only reason to do this would be if you were running multiple computers in an observing field and you wanted a central server, in which case all you had to do is what you are currently doing as you described.

So yes, I think we would want to mainly implement the local library version.

rlancaste commented 2 years ago

One important point for you, I made a tester program in stellarsolver, which is VERY nice for testing and comparison of various solving methods. That tester program is available on Mac, Windows, and Linux. You could use it to test your solver and compare its solving speed, its solution results, and various other items and use it to optimize your solver.

rlancaste commented 2 years ago

Once I integrate your solver, then you can use that program. I would guess you could just keep using the same executable and just update the library you are designing as you make changes to it.

rlancaste commented 2 years ago

Ok I downloaded your code and started poking around. I ran into a couple of issues.

The first issue I ran into is I don't see any clear instructions on how to build the code. I noted that you included binaries for Windows, Mac, and Linux which is very good, but it would be better if we could get some method to easily build it on the various systems so that we can get craft to include it in the daily builds and so the Linux PPAs could include it easily. I guess you can just have them install a binary, but I think it might be better if there was a way to build it from the command line or on a server. That way users can build the latest code and test it on their systems. Also, you might not have binaries for every system out there. I don't know if this would be possible since this is C# and .net? Do you need Windows, Visual Studio, and/or .net to build the code?

The second question is about using this program inside a C++ application. It looks like you have a both a CLI executable and a "nupkg" library file. It is my belief that the most desirable thing would be to use a library instead of the command line executable, and to just pass image buffers and data back and forth (This is what I am doing with stellarsolver), but is it possible to use this library from C++ or would you need C# and visual studio to do it? If necessary, we could use the command line and access Watney the same way we access ASTAP and local astrometry.net. But if it could be done as a library, I think that is highly desirable. If it is a library, there will be minimal file writing which is a priority for raspberry pis.

Jusas commented 2 years ago

The first issue I ran into is I don't see any clear instructions on how to build the code. I noted that you included binaries for Windows, Mac, and Linux which is very good, but it would be better if we could get some method to easily build it on the various systems so that we can get craft to include it in the daily builds and so the Linux PPAs could include it easily. I guess you can just have them install a binary, but I think it might be better if there was a way to build it from the command line or on a server. That way users can build the latest code and test it on their systems. Also, you might not have binaries for every system out there. I don't know if this would be possible since this is C# and .net? Do you need Windows, Visual Studio, and/or .net to build the code?

Building the binaries: this is easy, and I guess I've not included any instructions because I assumed it would be straightforward for anyone who's been developing dotnet projects. Your run-of-the-mill developer would download the sources and open them in Visual Studio, VSCode, Rider or any IDE of their choosing and just run "build solution". But it doesn't require any IDE for building, all you need is the dotnet 6 sdk. You can do that in Windows, Linux and Mac (any platform/arch that can run dotnet). Depending on which project you want to build, it's a simple as running a dotnet command:

dotnet publish -r <architecture> -c Release -o /path/to/bin --self-contained true /path/to/project.csproj

That'll build you a ready to run self-contained binary which is targeted to run on the specified os/architecture.

The second question is about using this program inside a C++ application. It looks like you have a both a CLI executable and a "nupkg" library file. It is my belief that the most desirable thing would be to use a library instead of the command line executable, and to just pass image buffers and data back and forth (This is what I am doing with stellarsolver), but is it possible to use this library from C++ or would you need C# and visual studio to do it? If necessary, we could use the command line and access Watney the same way we access ASTAP and local astrometry.net. But if it could be done as a library, I think that is highly desirable. If it is a library, there will be minimal file writing which is a priority for raspberry pis.

I don't think it'll be possible to use the library directly from C++. It's a dotnet library, "managed code", which requires the CLR (Common Language Runtime) to run. Doing it the other way around is possible, i.e. invoking a native C++ library from managed code - but calling managed code from unmanaged code does not really seem achievable in any reasonable way. The library was meant as a pluggable solution for other dotnet projects like SharpCap for example. So right now I'd go with the way you call ASTAP.

However maybe what we could do is pass data using a pipe from the command line? Would that work? It would need to be in a string format of course. EDIT: I suppose we can pipe binary data just as well...

Jusas commented 2 years ago

Alright, I just pushed changes that allow reading the image from stdin when --from-stdin flag is present (when it's present, --image parameter is ignored), e.g. from command line:

whateverprogram | watney-solve blind --min-radius 0.5 --max-radius 8 --from-stdin

(Also fixed a pretty bad bug that always failed nearby solves when using initial info from FITS headers)

You can build this with for example:

dotnet build -c Release ./src/WatneyAstrometry.SolverApp/WatneyAstrometry.SolverApp.csproj -o ./bin

Or to get the self-contained executable, which is the form I use for distribution:

dotnet publish -c Release -r <arch> --self-contained true ./src/WatneyAstrometry.SolverApp/WatneyAstrometry.SolverApp.csproj -o ./bin

Just replace <arch> with your machine architecture (probably win-x64, linux-x64 or osx-x64)

This allows piping the image bytes directly to the solver - maybe this'll be a satisfactory solution for the file writing problem?

rlancaste commented 2 years ago

Ok, so yes I am thinking I agree with you, the best solution is to use this like we use ASTAP, to call the external program with arguments, since I have no idea how to make it possible to call it as a library from C++. I have no experience with dotnet or C#, but lots of experience with Java, python, and C++. Pretty much all the code for StellarSolver, INDI, and for KStars is C++ or C.

And yes I completely understand that a developer could install dotnet on their own machine, run your script to compile the program and produce the executable. I would definitely appreciate you adding some instructions for how to do that. But then the next challenge to that is how to get the automated build systems we use to do it. As I said, one option might be to just get them to install the final executable, but of course that would mean somebody would have to regularly update that. For craft, with which I have much more experience, I think that will be fairly easy since we can just command the system to download your latest archive and install that executable. But I don't know how it would work on the PPA or on the CI build systems. I will ask.

For the command line options, can I ask that you try to add at least as many options as ASTAP? Also a very huge performance gain in StellarSolver and other programs is gained by doing star extraction with SEP. Can you make sure there is an option to send a star list instead of just an image?

rlancaste commented 2 years ago

If you don't want to add support for star lists, that is ok too. Han had added that support when I asked him to do so while I was first developing StellarSolver. But then he decided to remove it.

Jusas commented 2 years ago

For the command line options, can I ask that you try to add at least as many options as ASTAP? Also a very huge performance gain in StellarSolver and other programs is gained by doing star extraction with SEP. Can you make sure there is an option to send a star list instead of just an image?

Sure, what format are we talking about? XYLS (FITS with a binary table)? The XYLS here https://github.com/dstndstn/astrometry.net/tree/main/demo are these what you're referring to? The needs of the solver are pretty much the same as with ASTAP; star X, Y positions + brightness (flux?) and image width and height.

rlancaste commented 2 years ago

Yes, that is exactly what I mean. If you change the settings in stellarsolver to extract with internal SEP and solve with external astrometry.net, then you will get an xyls file and that gets solved by astrometry.net. If you turn off the option to cleanup temp files, you will be able to keep those files and see how they work.

Jusas commented 2 years ago

Alright, I just did exactly that and got myself a .xyls file. After inspecting it there's still the issue of not getting the image dimensions as a part of the file - these would then need to be passed as command line parameters as the solver needs them.

rlancaste commented 2 years ago

Yes, that is what we do with astrometry.net

rlancaste commented 2 years ago

but an xyls file is way way smaller than a fits file or a raw image file

rlancaste commented 2 years ago

For example, with online solving with nova.astrometry.net, if you are in an observing field especially if you are using cell phone data, it is MUCH better to upload an xyls file than the full image

Jusas commented 2 years ago

I just pushed a new commit which adds the support for .xyls inputs to the CLI solver.

Usage example:

./watney-solve blind --max-radius 8 --min-radius 1 --xyls F:\Dev\StellarSolver\temp\externalSextractorSolver_2.xyls --xyls-imagesize 4479x3375

When providing --xyls (xyls file path) or --xyls-stdin (xyls from stdin) you must also provide --xyls-imagesize.

Also renamed the --from-stdin flag to --image-stdin to better group the arguments.

rlancaste commented 2 years ago

Ok, I started working on adding support for this. Is there a place to get an updated binary, or do I have to either wait for you to build it or try to do that myself using your instructions?

rlancaste commented 2 years ago

Also a question, for the database files, they download to separate folders, do they all have to go in the same directory or can they stay in their sub folders in the bigger directory?

rlancaste commented 2 years ago

Also, would these parameters look like they are formatted well? /Users/rlancaste/Projects/watney-solve/watney-solve nearby -r 3.78231 -d 24.1156 -s 15 -m -f 0.762852 -i /private/var/folders/_t/ntxs0hp56b31tsp0_9t3rttm0000gn/T/externalSextractorSolver_4.fits -o /private/var/folders/_t/ntxs0hp56b31tsp0_9t3rttm0000gn/T/externalSextractorSolver_4.ini -w /private/var/folders/_t/ntxs0hp56b31tsp0_9t3rttm0000gn/T/externalSextractorSolver_4.wcs

rlancaste commented 2 years ago

and these as well for blind? /Users/rlancaste/Projects/watney-solve/watney-solve blind --min-radius 0.2 --max-radius 15 -i /private/var/folders/_t/ntxs0hp56b31tsp0_9t3rttm0000gn/T/externalSextractorSolver_5.fits -o /private/var/folders/_t/ntxs0hp56b31tsp0_9t3rttm0000gn/T/externalSextractorSolver_5.ini -w /private/var/folders/_t/ntxs0hp56b31tsp0_9t3rttm0000gn/T/externalSextractorSolver_5.wcs

rlancaste commented 2 years ago

Those are actually for the same image, not blind and blind

rlancaste commented 2 years ago

Actually, 0.2 for a min made it fail, I had to use 0.5 to get it to succeed

rlancaste commented 2 years ago

Ah, I just found there was an issue with my nearby parameters as well and its why it was not solving. The nearby parameters RA value is not in hours like it is in ASTAP, it is in Degrees. Please update your documentation since it doesn't say the units.

Jusas commented 2 years ago

Also a question, for the database files, they download to separate folders, do they all have to go in the same directory or can they stay in their sub folders in the bigger directory?

They're supposed to be extracted to the same directory. I suppose I haven't said that anywhere.

Also, would these parameters look like they are formatted well? /Users/rlancaste/Projects/watney-solve/watney-solve nearby -r 3.78231 -d 24.1156 -s 15 -m -f 0.762852 -i /private/var/folders/_t/ntxs0hp56b31tsp0_9t3rttm0000gn/T/externalSextractorSolver_4.fits -o /private/var/folders/_t/ntxs0hp56b31tsp0_9t3rttm0000gn/T/externalSextractorSolver_4.ini -w /private/var/folders/_t/ntxs0hp56b31tsp0_9t3rttm0000gn/T/externalSextractorSolver_4.wcs

Looks valid to me.

and these as well for blind? /Users/rlancaste/Projects/watney-solve/watney-solve blind --min-radius 0.2 --max-radius 15 -i /private/var/folders/_t/ntxs0hp56b31tsp0_9t3rttm0000gn/T/externalSextractorSolver_5.fits -o /private/var/folders/_t/ntxs0hp56b31tsp0_9t3rttm0000gn/T/externalSextractorSolver_5.ini -w /private/var/folders/_t/ntxs0hp56b31tsp0_9t3rttm0000gn/T/externalSextractorSolver_5.wcs

Also looks valid.

Ah, I just found there was an issue with my nearby parameters as well and its why it was not solving. The nearby parameters RA value is not in hours like it is in ASTAP, it is in Degrees. Please update your documentation since it doesn't say the units.

Ah, very true. Completely forgot it there.

Ok, I started working on adding support for this. Is there a place to get an updated binary, or do I have to either wait for you to build it or try to do that myself using your instructions?

I spent this evening setting up a "bleeding edge" CI pipeline with Github Actions. I'll get to documenting the build process in general sometime soon, but right now the CI pipeline will create new builds per every push I make to the master branch that affects the CLI solver.

The latest release is automatically updated to this release: https://github.com/Jusas/WatneyAstrometry/releases/tag/bleeding-cli

The watney-solve-bleeding-latest* packages will always be the latest ones. In addition it creates packages named watney-solve-bleeding-<date_and_time>-<commit_hash>* and I'll be deleting old ones manually when they start to add up too much - at the moment I think it may be of value to keep previous builds available.

rlancaste commented 2 years ago

Awesome, I have gotten it working in StellarSolver for blind solving and informed solving with your built in method for star extraction. I probably still need to go over the parameters to see that I am using them all well, but its working. Next I will check out the xyls input function you added and see how that works.

Jusas commented 2 years ago

A small status update: I've been playing with the database file format a bit and trying to squeeze out more performance and perform less reads. I realized I could save a few more bytes per quad while sacrificing some accuracy on the quad ratios, which actually makes the files 22% smaller, and I also separated the header data from individual database cells and put it into an index file instead. Previously with the full database you were looking at reading the headers of 2030 files (5 sets of 406 files) - now it's just 5 files, and read in parallel. This basically dropped the DB initialization duration from ~400ms to ~80ms on my machine and I bet the effect on slow setups like RPI is even more significant. Also due to the database being smaller, overall less data is being read per quad, resulting in a bit faster solve times as well, I haven't benchmarked this yet but I have a feeling that it'll be noticeable.

I still need to run some more tests to get some performance data and make sure the changes didn't break anything before I merge this to master though.

rlancaste commented 2 years ago

Very good, I have been working on other things in StellarSolver recently. But I think the support for WatneySolver is pretty good now. It its still in the version2.0stuff branch. Our plan is to implement the change to KStars to support it at the same time that I update StellarSolver to 2.0. At that time, KStars will support Watney

Jusas commented 2 years ago

Been working on the performance again this week. After a bit of testing it looks like the database changes weren't as impressive as I thought - the side effects of having to run overall more calculations due to "false positives" produced by the lesser accuracy in quad ratios pretty much ate the performance boost in blind solves but in the end I found a pretty good compromise, and producing a 18% smaller database while keeping performance pretty much the same. However I finally got to address my old TODO notes regarding caching and oh boy, did that make a huge difference with blind solves! Running the profiler and realizing some obvious pain points I implemented some caching when reading the database. The impact was huge. Comparing to old blind solves on RPi4 I'm looking at 30-80% faster (!) blind solve times (the more stars and the smaller the field, the bigger the impact). This actually makes blind solves somewhat bearable even on budget hardware. Nearby solves were affected too, on a similar level, maybe 50% faster (much of this boost is likely from reduced file reads, as the new database format has separate compiled index files).

So yeah, happy times! :)

I also tested StellarSolver and the version2.0stuff branch, confirming that things work. Going towards the adoption in KStars, there are a few things I'd like to make note of in regards to CLI arguments:

I noticed the default StellarSolver --max-radius arg was set to 15. This was actually good, because it made me notice some things:

Most of these could just be set as a list of CLI args in a single input text field, it would suffice for a minimal UI, although I'd separate nearby and blind args to different fields as they are different parameter sets altogether.

Anyway is it Jasem who ultimately puts the UI together in KStars or is it you? I could mock up some UI ideas if you think that'll be useful - but I think my C++ is way too rusty to give a code contribution in this regard.

rlancaste commented 2 years ago

https://indilib.org/forum/ekos/11388-announcing-stellarsolver-2-0.html

rlancaste commented 2 years ago

So for the UI changes and StellarSolver changes, we all do it. I just added several UI changes to KStars to support the new version of StellarSolver I made.

rlancaste commented 2 years ago

So I should explain that I created an object called parameters, which is a set of options, and I created this concept called options profiles for KStars. The different star extractors and solvers have many many options in different places such as in files, in settings that get passed when you solve, and in files you pass to them. My goal was to simplify all that and get it into one place that works for all the solvers. Then I wanted to make a set of profiles that are specific to certain tasks, such as plate solving with reflectors that have larger stars, plate solving big star fields, or start extractions for galaxies, etc. Most of the options you mentioned I believe already have a place in my options profiles. If I hard coded anything in stellarsolver, we can work to improve that to match it to the profiles. This work that I just did should be considered preliminary support for your Watney solver and we will work to improve it.

rlancaste commented 2 years ago

KStars and INDI have many dozens of contributors and they all do their part to make it better.

rlancaste commented 2 years ago

I will look at these options you mentioned and see what I can do with them.

rlancaste commented 2 years ago

Please check if this improves things. https://github.com/rlancaste/stellarsolver/commit/2954bc1e0d45a1f6e54865df0c801a88343b4954

Jusas commented 2 years ago

Ok, I took a good look and:

Again, generally the approach of average that you've used works fine. For the first release I'm quite happy with it. Only if the range gets big then using the field-radius-range makes sense. Maybe if the low-high range is 3 degrees, this starts to be useful as the average gets too rough. To be truthful, I haven't really tested out the thresholds so the 3 degrees is just a rough guess. With smaller ranges the --lower-density-offset and --higher-density-offset compensate enough to find a solution. This is something I should look at sometime, so now this note was more like a heads-up for you.

Truly thanks for all the effort you're taking! :+1: I'm hoping that once you've got things in place and the StellarSolver 2.0 comfortably set up in KStars, I could then start to contribute properly to Watney related things doing minor tweaks where I find them useful. I'm slowly getting more comfortable with C++ again after a long break...

rlancaste commented 2 years ago

So to clarify, the reason for the code that you cited above where I averaged the scale low and scale high values, the reason for that is because KStars has a pretty good idea of the image scale, then most of the solvers accept scale low and scale high values, so KStars calculates a range from its best guess and gives those values. But ASTAP and now Watney don't accept low and high scales the same way the other solvers do, they want the actual guess for the scale. So this code is just converting back to KStars' initial estimate of the scale.

rlancaste commented 2 years ago

ok I just updated again. Please let me know if there is anything else

Jusas commented 2 years ago

So to clarify, the reason for the code that you cited above where I averaged the scale low and scale high values, the reason for that is because KStars has a pretty good idea of the image scale, then most of the solvers accept scale low and scale high values, so KStars calculates a range from its best guess and gives those values. But ASTAP and now Watney don't accept low and high scales the same way the other solvers do, they want the actual guess for the scale. So this code is just converting back to KStars' initial estimate of the scale.

Yeah, and that is fine.

Looks good :+1:

Jusas commented 2 years ago

Found two new issues:

Error in Watney parameters when calling a blind solve:

/home/jusasi/watney/watney-solve blind --min-radius 0.5 --max-radius 15 -f 0.580652 -i /tmp/externalExtractorSolver_3.fits --max-stars 300 --use-parallelism -o /tmp/externalExtractorSolver_3.ini -w /tmp/externalExtractorSolver_3.wcs --log-file /tmp/astrometryLog.txt

The parameter -f is only for nearby solves, so that needs to be dropped here.

The parameter --use-parallelism is also not a flag; it's possible to set the default to either be true or false in the configuration file, and therefore when using the parameter the value must be specified to either true or false. This is my fault for not communicating it.

Also as I was testing just now I noticed the Ekos Scheduler is broken in master (nothing to do with your stuff, just a heads up).

Jusas commented 2 years ago

Oh nevermind about the scheduler note, problem between chair and keyboard...

rlancaste commented 2 years ago

Ok please check to see that it is updated properly now

Jusas commented 2 years ago

Oh, there's a bug, you have:

    if(m_ActiveParameters.multiAlgorithm == SSolver::NOT_MULTI)
        solverArgs << "--use-parallelism false";
    else
        solverArgs << "--use-parallelism true";

When you should have:

    if(m_ActiveParameters.multiAlgorithm == SSolver::NOT_MULTI)
        solverArgs << "--use-parallelism" << "false";
    else
        solverArgs << "--use-parallelism" << "true";
rlancaste commented 2 years ago

Quite right, it should be fixed now. Also note I was getting a crash in the latest Watney. First it didn't recognize the -f parameter at all anymore, but even after I changed to the full name, I'm getting this:

Unhandled exception. System.AggregateException: One or more errors occurred. (Object reference not set to an instance of an object.) ---> System.NullReferenceException: Object reference not set to an instance of an object. at WatneyAstrometry.Core.QuadDb.CompactQuadDatabase.<>c.b__8_0(QuadDatabaseCellFileSet x)

Jusas commented 2 years ago

Confirmed, just pulled the latest and ran, it works now.

Quite right, it should be fixed now. Also note I was getting a crash in the latest Watney. First it didn't recognize the -f parameter at all anymore, but even after I changed to the full name, I'm getting this:

Unhandled exception. System.AggregateException: One or more errors occurred. (Object reference not set to an instance of an object.) ---> System.NullReferenceException: Object reference not set to an instance of an object. at WatneyAstrometry.Core.QuadDb.CompactQuadDatabase.<>c.b__8_0(QuadDatabaseCellFileSet x)

Interesting - are you running the latest version, and do you have the latest quad database downloaded? I'm testing using the latest, and it works for me. See https://github.com/Jusas/WatneyAstrometry/releases/tag/v1.1.1 and https://github.com/Jusas/WatneyAstrometry/releases/tag/watneyqdb3

rlancaste commented 2 years ago

ah, so the database files changed? I upgraded and now it no longer works?

Jusas commented 2 years ago

From 1.0 -> 1.1 the database format changed, the indexes were moved to separate files and the data entry size changed. 1.1 can't use the old database version 2 format and 1.0 can't use the new db version 3 format since the changes were so drastic.

rlancaste commented 2 years ago

Gotcha, ok it works now

rlancaste commented 2 years ago

I would recommend some kind of check to see if stuff is compatible, maybe have the information that you need to upgrade your database pop up instead of an error.