Fix: try to improve the visibility criteria

[fjebaker]

Basically, the conclusion of investigating #160 is that the root finder thinks it can see emission radii that are actually obscured. It's amazing just how big of an impact this really has, but tweaking the old visibility tolerance parameter helped the two curves line up in places. So it's clear we need a better visibility criteria: the question is just what?

I tried to make the _is_visible function more robust in this PR. Instead of just checking the proximity of a trajectory, traced with the full thick disc, to the target emission radius (which is biased towards concluding visible at small radii), the first simple change is to check the Cartesian distance between the target point and the geodesic endpoint.

This isn't however always enough, and requires recomputing the geodesic too (which should be treated at the $< \epsilon$ level as probabilistic). Another method that I have tried is to compute the vector normal to the surface of the disc via automatic differentiation, and then check if the velocity vector at the endpoint of the geodesic, $\vec{v}$, satisfies

$$ \vec{n} \cdot \vec{v} < 0 $$

as a visibility critera. Since both tangent vectors belong to the same space of forms in the Kerr spacetime, the tangent space can be treated as locally Euclidean (Minkowski), and therefore this is equivalent to asking whether the geodesic came from behind the disc or not.

However, neither of these two things (seperately or in tandem) are enough to correct the lineprofile differences. The state is slightly improved, and by tweaking tolerances once can more or less force the line profiles to become very similar, albeit noisy, just as before.

This introduces a slightly metaphysical question however: if we regard the transfer funciton method as equivalent to an image with infinite resolution, then it will be able to see small patches of the disc that our finite resolution render will not. Are these emissions physical? Should these flux contributions be included, and the binning method disregarded as unable to appropriately evaluate in the limit of infinite resolution? Or do we conclude that the transfer function method is giving too much weight to portions of the disc that would lend miniscule contributions to the overall flux profile?

I am still unable to decide which of the two methods is "correct". And although we have discovered the bug, it is unsatisfying that an improved visibility criteria is unable to improve the lineprofiles unless the visibility criteria lends myopia to the method. It's kind of like saying "yes, the transfer function method agrees, but only if you or the algorithm squint".

[fjebaker]

If this PR doesn't break any tests, I will merge it and focus on some parts of the code in dire need of refactoring, along the lines of #163 and #164 .

[codecov-commenter]

Codecov Report

Patch coverage: 65.90% and project coverage change: +0.12% 🎉

Comparison is base (ba5ff32) 69.15% compared to head (156458e) 69.27%.

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #165      +/-   ##
==========================================
+ Coverage   69.15%   69.27%   +0.12%     
==========================================
  Files          66       66              
  Lines        2707     2734      +27     
==========================================
+ Hits         1872     1894      +22     
- Misses        835      840       +5     

Files Changed	Coverage Δ
src/geometry/discs.jl	4.54% <ø> (ø)
src/plotting-recipes.jl	0.00% <0.00%> (ø)
src/solution-processing.jl	48.78% <0.00%> (-1.22%)	⬇️
src/tracing/precision-solvers.jl	69.44% <69.23%> (+1.40%)	⬆️
src/geometry/discs/thick-disc.jl	78.57% <84.61%> (+11.90%)	⬆️
src/corona/samplers.jl	92.10% <100.00%> (+0.43%)	⬆️
...ransfer-functions/cunningham-transfer-functions.jl	97.29% <100.00%> (-0.06%)	⬇️

... and 1 file with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[fjebaker]

Nevermind I found the actual bug:

[fjebaker]

Straight out of the old catechism. Soon as one gives up all hope: providence.

The Jacobian function wasn't being given the thick disc, and was calculating the Jacobian over the datum plane instead. It's amazing that that actually makes a difference when you consider what's going on, but yeah. Wow.