Closed cgarling closed 4 months ago
Attention: Patch coverage is 89.75904%
with 17 lines
in your changes missing coverage. Please review.
Project coverage is 88.06%. Comparing base (
c45e31e
) to head (587bc52
).
Files | Patch % | Lines |
---|---|---|
src/StarFormationHistories.jl | 89.37% | 17 Missing :warning: |
:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.
Fixes #30.
This is a major PR that separates the kernels we support to describe the 2-D probability distributions of observed magnitudes given intrinsic magnitudes into
RealSpaceKernel
s andPixelSpaceKernel
s depending on whether their fields (centroids, widths, etc.) are defined in magnitude (real) space or the pixel space of the Hess diagram matrix.This was added to enable definition of the covariant kernel
GaussianPSFCovariant
which is easier to describe in real space coordinates than pixel space coordinates. This kernel is valid for Hess diagrams where the y-axis magnitude also appears in the x-axis color. Addition of this kernel greatly improves modelling accuracy for these types of Hess diagrams. A future refactor may be able to convert this into aPixelSpaceKernel
, but the extra code necessitated by the implementation ofaddstar!
for subtypes ofRealSpaceKernel
adds very little overhead (~200 ns per call) and so this is not a priority.A new example
examples/templates/kernels_example.jl
has been added showing the performance of this functionality. The output of this script is spliced into a new documentation page under thefitting/internals
heading.New tests, including of
partial_cmd_smooth
, have been added. Callingpartial_cmd_smooth
will necessarily call most of the other functions in the main "template creation" process. Many of these methods presently lack their own unit tests but this is low priority for now.There was also an issue with the old
data/isochrone.txt
which has now been fixed.I added VectorizationBase.jl to deps to get
AbstractSIMD, verf
for this PR but now I don't think it is truly necessary. However, this is a dependency of LoopVectorization.jl which I do need and so we would have to install it anyway. For now I'm fine keeping it but might remove in the future.