Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Enhancement: Eccentricity #117

Open
schackey opened this issue Jul 18, 2023 · 3 comments
Open

Enhancement: Eccentricity #117

schackey opened this issue Jul 18, 2023 · 3 comments

Comments

@schackey
Copy link
Contributor

What do you think about using eccentricity as opposed to minor length as a threshold in detection blocks?

I feel that then to rule out small stars one can use min_area, and using eccentricity rather than minor_length would then be a pure parameter to control the allowed ellipticity of stars. Using minor length seems to try to do both at the same time, and sometimes i then find it difficult to constrain the parameters correctly to only detect the stars i want. What do you think?

I will include it in the branch i'm building, if you like the idea!

@lgrcia
Copy link
Owner

lgrcia commented Jul 18, 2023

Yes I think that's definitely a good idea. I thought this was well made but its not at all.

As you say: when using a PointSourceDetection block we should filter and get only truly point sources (based on eccentricity). This idea was implemented in the AutoSourceDetection block (see the algorithm here) but I don't see this enforced on any of the shape-specific detection blocks. With that we may be able to get rid of the minor_length.

Here is a proposed implementation:

  • we replace minor_length by eccentricty_bound in the base _SourceDetection class
  • we filter for sources in the clean method, so that only sources with an eccentricity within eccentricty_bound are kept

This way we inforce shape-specific detection blocks to be truly shape-specific :). What do you think?

@schackey
Copy link
Contributor Author

Perfect, yes that's exactly how I have implemented it so far! I will clean it up and pull request soon.

@lgrcia
Copy link
Owner

lgrcia commented Jul 18, 2023

That's awesome, thanks! 🙏🏼

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants