Tickets/dm 4768 #12
Merged
Tickets/dm 4768 #12
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| (boost::format( | ||
| "initial parameter guess resulted in negative radius; used minimum of %f pixels instead." | ||
| ) % ctrl.minInitialRadius).str() | ||
| ); | ||
| ellipse = afw::table::QuadrupoleKey::addFields( | ||
| schema, | ||
| schema.join(prefix, ".ellipse"), |
This is just a convenience, but it's often very inconvenient not having it.
They're still disabled by default, and will never apply to the linear fit (just the linear fit for ellipse parameters).
This was really just writing out the noise-replaced image, which should be the responsibility of the measurement framework, not a particular algorithm. And it'll be difficult to keep with the new fit-region determination code.
After this change, CModel will use the shape slot and Kron radius (if available) to set the initial fit region instead of the detection Footprint, and it will put an upper bound as well as a lower bound on the final fit region from the initial fit. The net result seems to be greatly improved computational performance and a small decrease in the number of spuriously large objects.
We'll be using this shortly in a new prior class.
This new prior improves CModel in several ways, including robustness against bad deblends and computational performance, but it has not yet been verified that it does no harm, so it's not being used as the default yet.
Using a small dof as in the previous default produces tails that are much too broad, allowing objects to have catastrophically high ellipticities and wrecking the computational performance.
This is currently broken because the fit region isn't transformed, but it's better to explicitly fail than run with incorrect behavior. See DM-5405.
This is a far cry from complete documentation for the algorithm, which would be beyond the scope of this issue, but it does address some behavior that might be considered mildly surprising.
TallJimbo
force-pushed
the
tickets/DM-4768
branch
from
ffc7c13
to
de93f04
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No description provided.