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

Ellipticity validation #113

Merged
merged 10 commits into from
Jun 5, 2018
Merged

Ellipticity validation #113

merged 10 commits into from
Jun 5, 2018

Conversation

msimet
Copy link
Contributor

@msimet msimet commented May 30, 2018

This code contains the validation criteria for the ellipticity distribution test. A run of this test on the v4.* catalogs can be found here. Everything passes except for the late-type galaxies in proto-dc2_v4.5_rescale, where the 10th percentile is a little high (0.41 instead of <=0.4). I'm not sure that's actually important--I've left if for now so more knowledgeable people can advise me if that's expected or not. It would probably be okay to adjust that criterion if we needed to, however.

The actual validation criteria used here are:

  • min(e)>=0
  • max(e)<=1
  • 10th percentile < 0.4
  • 0.3 < 50th percentile < 0.7
  • 90th percentile > 0.6

This is tested for all morphologies separately and for the catalog as a whole; the returned float value is the total number of failed criteria. The criteria are currently hard-coded but the percentiles could be moved to config files if desired.

@aphearin
Copy link

Thanks for the update @msimet -

CC @dkorytov @evevkovacs - based on the updated DESCQA test Melanie posted above, it looks like the ellipticity model upgrade was applied to v4.4 and v4.5, but not v4.5_rescale.

Copy link
Member

@yymao yymao left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you @msimet! The change looks good. I wonder if it's better to specify validation_percentiles in the yaml config file as oppose to hard writing them in the code as they currently are?

@msimet
Copy link
Contributor Author

msimet commented May 30, 2018

I'd be happy to make that change if you think it's useful!

@yymao
Copy link
Member

yymao commented May 30, 2018

@msimet Yes, in particular because the config files will included in the web interface, it will make it easier for users to find out what validation criteria are used.

Thank you for making that change!

@evevkovacs
Copy link
Contributor

@msimet @aphearin @yymao Thank you Melanie. This looks great. FYI the new ellipticity model has not been applied to proto-dc2_v4.5_rescale. Another useful feature that could be added a print out the individual percentile values in the txt file.

@msimet
Copy link
Contributor Author

msimet commented Jun 2, 2018

I think I have implemented these changes--you can view the new outputs here.

#check overall ellipticity distribution
global_N = np.sum(np.sum(N_array, axis=1), axis=0)
number_passed, percentiles = self.validate_percentiles(global_N)
print("Percentiles for global distribution are:".format(morphology)+', '.join([" {:.3f} ({})".format(p, v) for p,v in zip(percentiles, self.validation_percentiles['percentiles'])]))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the first .format is not actually being used.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay, I think this is fixed now...

Copy link
Member

@yymao yymao left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, @msimet, for the quick fix! This looks good. I'll merge it on Tuesday.

@yymao yymao merged commit 317aa21 into LSSTDESC:master Jun 5, 2018
@yymao yymao mentioned this pull request Jun 14, 2018
4 tasks
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

Successfully merging this pull request may close these issues.

None yet

4 participants