#30 part: geodtest inverse12 S12

[stonylohr]

Assorted fixes. Added test exception for GeodTest.dat line 400001 inverse12, which Karney's code fails identically to ours.

This one has been slower going. There are clearly some remaining issues that should be addressed through further fixes in either main code or tests, one of which involves Lambda12 returning a value that differs at the 3rd significant digit for inputs that differ at the 13th digit or later. Other cases not yet explored at all. Anyway, I thought what I have so far was worth sharing. I can comment more in the coming days, but have to get going for now.

• [ ] I agree to follow the project's code of conduct.

• [ ] I added an entry to CHANGES.md if knowledge of this change could be valuable to users.

[michaelkirk]

bors try

[bors[bot]]

try

Build succeeded:

• tests

[michaelkirk]

That makes sense. Thanks for pursuing!

On Feb 8, 2021, at 21:52, stonylohr notifications@github.com wrote:

 @stonylohr commented on this pull request.

In src/geodesic.rs:

             let g = g.lock().unwrap();

let (s12_out, azi1_out, azi2_out, m12_out, _M12_out, _M21_out, S12_out, a12_out) = g.inverse(lat1, lon1, lat2, lon2); assert_approx_eq!(s12, s12_out, 8e-9); assert_approx_eq!(azi1, azi1_out, 2e-2); assert_approx_eq!(azi2, azi2_out, 2e-2); assert_approx_eq!(m12, m12_out, 5e-5);

• assert_approx_eq!(S12, S12_out, 3e10); // Note: unreasonable tolerance
• // Our area calculation differs significantly (~1e7) from the value in GeodTest.dat for
• // line 400001, BUT our value also perfectly matches the value returned by GeographicLib
• // (C++) 1.51. Here's the problem line, for reference:
• // 4.199535552987 0 90 -4.199535552987 179.398106343454992238 90 19970505.608097404994 180 0
• if line_num != 400001 {
• assert_approx_eq!(S12, S12_out, 3e10); // Note: unreasonable tolerance Looking back at this, I see that I was using a relative error to pick a "worst" line for S12, and it basically just picked the first one that had expected 0 and actual with a large value, which all end up with a relative error of 2. There are more with relative errors of 2, and some of those still have absolute errors in the same range as line 400001, so it's not clear that we can reduce the tolerance at all yet. Just to be safe, I plan to revise my "worst line" metrics to pick a separate worst line for absolute and relative error for any value where I think we care about relative error (I still think it makes sense for S12), and rerun with the updated metrics using code from both before and after the changes in this PR, to make sure that everything is really moving in the right direction. I don't expect to finish that review tonight.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub, or unsubscribe.

[stonylohr]

I finished an initial review, and I'm not seeing any sign that the changes in this PR add regressions that were hidden by using relative difference to pick a worst line for S12. But I'm also getting the sense that the last 100k lines of GeodTest.dat may have quite a few cases where the C++ Geodesic inverse returns a value that doesn't match S12 well. I'm guessing that GeodesicExact does better, but I haven't checked. Anyway, I may need to reformulate my approach to more efficiently identify and deal with these cases. It's also looking like the coming month or so may be a bit complicated for me. While I expect to find some time to continue, I expect my pace will slow for at least a while.

[michaelkirk]

bors r+

Though it doesn't completely resolve #30, I'll merge this as is since it fixes some clear bugs in the transcription and doesn't break anything.

Thanks @stonylohr!

[bors[bot]]

Build succeeded:

• tests