search

onekiloparsec / SwiftAA

The most comprehensive collection of accurate astronomical algorithms in (C++, Objective-C and) Swift.
http://www.onekiloparsec.dev/
MIT License
171 stars 31 forks source link

Use AA+ 2.44, with necessary modifications #111

Closed vincentneo closed 2 years ago

vincentneo commented 2 years ago

~Not tested.~ Did ran SwiftAATests, 7 failing out of 197. Do note that KPCAAElliptical now has Pluto commented out, as its removed from AAElliptical.

Maybe fixes #110

onekiloparsec commented 2 years ago

Thanks a lot for this large and great pull request! I'll have a look asap this week-end!

onekiloparsec commented 2 years ago

@vincentneo please allow me to write on your branch. Your PR is helping fixing a few things along the way, that's really cool (I am reviewing all the failing tests, fixing one after the other).

onekiloparsec commented 2 years ago

That's it, I've all tests green again.

vincentneo commented 2 years ago

@vincentneo please allow me to write on your branch.

btw, do I need to do anything to allow write access?

onekiloparsec commented 2 years ago

Yes: https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/working-with-forks/allowing-changes-to-a-pull-request-branch-created-from-a-fork

onekiloparsec commented 2 years ago

Well I don't know if you did something. On my side, not completely sure of how to use the GitHub CLI, I nonetheless tried to push my changes to that PR and tadaaa, here there are. Hence, I can now merge. And I will make a release right after it.

vincentneo commented 2 years ago

I haven’t had the chance to do anything just now, so I guess it does work by itself after all :)

onekiloparsec commented 2 years ago

Well, PR is merged, thanks you very much for your work. However, I am just unable to make a CocoaPod release for now. Too many cryptic compilation errors. Even the command swift test doesn't work. I am playing updating the Package.swift file, but to no avail.

onekiloparsec commented 2 years ago

It seems the AA+ author updated some stuff. One needs to investigate how he changed his way of compiling... What I don't understand is that Xcode is able to compile and run the tests. Yet, impossible to go beyond that.

vincentneo commented 2 years ago

@onekiloparsec

I had to change ObjCAA to compile using C++ 17 because of a change in AA+.

In AAVSOP87D_EAR.cpp,

PJN / 22-04-2020 1. Reworked C arrays to use std::array.

I believe this change, which uses constexpr, required it to be compiled using C++ 17, at least when I couldn't compile it, I searched around and C++ 17 seemed to be the solution as it worked. (I am not a C/C++ developer)

Screenshot 2022-07-25 at 10 47 39 PM

My guess is swift test internally compiles C++ code at a lower C++ dialect, hence failure, and CocoaPods is just using the default behaviour.

This change in this answer may help in CocoaPods deployment, if the C++ issue is the reason: https://stackoverflow.com/a/63233322

EDIT:

one example of if you compile with GNU++ 14 set instead Screenshot 2022-07-25 at 10 55 44 PM

onekiloparsec commented 2 years ago

Thanks. I'll check that asap.

onekiloparsec commented 2 years ago

Still struggling a bit (pod spec lint still fails). I'll be off in vacations for a week, and I'll restart right after that.

vincentneo commented 2 years ago

I just realised you haven't created a release (2.4.0) on GitHub, maybe its that. (in case if the lint error was due to fatal: Remote branch 2.4.0 not found in upstream origin)

Anyways, just ignore this and enjoy your vacation first! I am just leaving this comment down, in case I forget in a week's time.