Detector update

[StackEnjoyer]

This PR addresses the following enhancements and issues:

• Detector update #29
• Split the test suite into smaller files #28
• Photodetector surface coordinates #36

Overall, this PR is a major breaking change in regards to the detector API, hence the update to v0.11.0

ToDo's

• update detector documentation
• consider post-solve kinematic change warning
• optional: implement electric_field fct. for PolarizedRay

[codecov]

Codecov Report

❌ Patch coverage is 76.03306% with 58 lines in your changes missing coverage. Please review.
⚠️ Please upload report for BASE (master@7a0ab61). Learn more about missing BASE report.
⚠️ Report is 1 commits behind head on master.

Files with missing lines	Patch %	Lines
src/OpticalComponents/Detectors/DetectorUtils.jl	72.83%	22 Missing ⚠️
src/OpticalComponents/Detectors/Detector.jl	73.07%	14 Missing ⚠️
src/Utils/MiscUtils.jl	18.18%	9 Missing ⚠️
...rc/OpticalComponents/Detectors/FieldCalculation.jl	88.40%	8 Missing ⚠️
src/OpticalComponents/Detectors/SpotDiagram.jl	50.00%	3 Missing ⚠️
src/Utils/LinearAlgebraUtils.jl	89.47%	2 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff            @@
##             master      #37   +/-   ##
=========================================
  Coverage          ?   71.57%           
=========================================
  Files             ?       57           
  Lines             ?     2881           
  Branches          ?        0           
=========================================
  Hits              ?     2062           
  Misses            ?      819           
  Partials          ?        0           

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

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[StackEnjoyer]

@TacHawkes there is something weird going on with the new "psf" intensity calculation. Tests keep failing randomly, I am wondering if it might be due to the random support vector in UniformDiskSource influencing the psf result

[TacHawkes]

@TacHawkes there is something weird going on with the new "psf" intensity calculation. Tests keep failing randomly, I am wondering if it might be due to the random support vector in UniformDiskSource influencing the psf result

I have added a commit which makes normal3d deterministic. The alternative would be to increase the atol threshold slightly.

[TacHawkes]

Some thoughts from review.

1. What about an "auto-reset" for detectors. I find it a little bit dangerous to expect the user to empty! on resolving. Something like this maybe?

function solve_system!(sys::System, beam; reset_detectors::Bool = false)
    if reset_detectors
        for obj in sys.objects
            if obj isa Detector
                empty!(obj)
            end
        end
    end
    return BeamletOptics.solve_system!(sys, beam)  
end

2. Just to be sure: You want the Detector to fix its type (currently the Vector is a UnionAll <: AbstractDetectorHit) upon first hit, so mixed hits are not possible. This is the intended behavior? Correct?

3. I have added one forgotten part of the docs to use the new API.

[TacHawkes]

See my other comment.

[StackEnjoyer]

Review

1. What about an "auto-reset" for detectors. I find it a little bit dangerous to expect the user to empty! on resolving.
2. Just to be sure: You want the Detector to fix its type (currently the Vector is a UnionAll <: AbstractDetectorHit) upon first hit, so mixed hits are not possible. This is the intended behavior? Correct?
3. I have added one forgotten part of the docs to use the new API.

1. In general I am in favor of this idea. I have been thinking about adding a generic initialize! API in order to do some housekeeping before running solve_system!, e.g.

initialize!(::AbstractObject) = nothing
initialize!(d::Detector) = empty!(d)

There are some use cases where a user might want to use the overwrite feature, e.g. multiple runs of solve_system! with different beams on a single system. This could be a toggleable feature as you suggest.
2. Yes this is correct. Cross-mixing is strictly forbidden currently.
3. Thx

@code_warntype checks

I have run @code_warntype with the following results:

• interact3d(::Detector) type-unstable (red) as expected, but this is a price I am willing to pay for this API
• spot_diagram / electric_field / intensity: in general these calls are type-unstable for e.g. electric_field(pd) but the underlying call to electric_field(pd, hits(pd)) appears to be type-stable except for some type combos (worth looking into)

I think this is fixable in a quick joint session @TacHawkes

[TacHawkes]

I tried to have a look into the second case. Cthulhu and 1.12 seem to be a bit broken but in general the type inference seems to struggle quite a bit, though the code looks fine. I will have a more in-depth look later. We might need some more function barriers to allow the compiler to optimize better. There are ::Any inferences at weird places.

[StackEnjoyer]

The type of pd.hits has been specified to Union{Nothing, RayHit{T}, etc...} rather than <:AbstractDetectorHit. This seems to have pleased the Julia type stability gods, at least from a static code analysis view. There is however still an issue here:

BeamletOptics.jl/src/OpticalComponents/Detectors/DetectorUtils.jl

Line 127 in 68a39b7

if center == :centroid

Is this an issue worth solving @TacHawkes ? Otherwise I will merge this PR.