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

Img bkg tests #104

Merged
merged 44 commits into from
Aug 29, 2018
Merged

Img bkg tests #104

merged 44 commits into from
Aug 29, 2018

Conversation

fjaviersanchez
Copy link
Contributor

@fjaviersanchez fjaviersanchez commented Apr 10, 2018

This pull request tries to incorporate tests on images. Documentation about the tests is still missing and some changes in the validation data are still needed but, hopefully, I'll have this working by tomorrow.

@fjaviersanchez
Copy link
Contributor Author

I managed to make the two tests included work. See here: https://portal.nersc.gov/project/lsst/descqa/v2/?run=2018-04-11_1&catalog=focal_plane_0_test

and here:

https://portal.nersc.gov/project/lsst/descqa/v2/?run=2018-04-11_14&test=quick_bkg_pk

Right now, the configurations just pull images from one directory (same as in the case of the instance catalog). Ideally, we would like that the program goes over all the visits available and repeats these tests. A different approach may be just create 10-15 config files with different visits, randomly selected. I think this might be sufficient.

@katrinheitmann
Copy link

katrinheitmann commented Apr 11, 2018 via email

@katrinheitmann
Copy link

katrinheitmann commented Apr 11, 2018 via email

@fjaviersanchez
Copy link
Contributor Author

fjaviersanchez commented Apr 11, 2018

@katrinheitmann Thanks for the comments. I think that you shouldn't be worried because I think that the raft 34 is on the edge of the focal plane so you expect some vignetting and that's the power that we are seeing. The raft that should be compatible with test 30 is R22. This can be changed in the configuration file. Rafts 12, 13, 21, 23, 31, 32, and 33 should also be fine/close. I chose the worst one... Please correct me if I am wrong @SimonKrughoff @jchiang87

@jchiang87
Copy link

Raft 11 is also one of the central 9 rafts.

@fjaviersanchez
Copy link
Contributor Author

@katrinheitmann I am suspecting of a bug in the power-spectrum code because I checked with a central raft and the power is still really high... https://portal.nersc.gov/project/lsst/descqa/v2/?run=2018-04-12_1&test=pk_img

@fjaviersanchez
Copy link
Contributor Author

fjaviersanchez commented Apr 12, 2018

Fixed: https://portal.nersc.gov/project/lsst/descqa/v2/?run=2018-04-12_10&test=pk_img (the blue line is perfectly below the orange because I am using the file from test 9 to check that the code is working).

And this is checking a different exposure: https://portal.nersc.gov/project/lsst/descqa/v2/?run=2018-04-12_12&test=pk_img

@yymao
Copy link
Member

yymao commented Aug 16, 2018

@fjaviersanchez I realized there are too many different things in this PR... can you do the following?

  • open a different PR for descqa/configs/readiness_instance.yaml --- this one looks fine and I can get it merged quickly.
  • remove descqa/BiasVersusRedshift.py, descqa/configs/bias_test.yaml, and descqa/configs/readiness_instance.yaml from this PR

Thanks!

@fjaviersanchez
Copy link
Contributor Author

Thanks @yymao. I implemented the changes that you suggested. By the way, I would suggest not to run this test on the full focal plane without rebinning (it takes a very long time to run).

@yymao
Copy link
Member

yymao commented Aug 27, 2018

@fjaviersanchez I've proposed some more changes by submitting a PR to your PR: fjaviersanchez#1

yymao
yymao previously approved these changes Aug 28, 2018
@yymao
Copy link
Member

yymao commented Aug 28, 2018

@fjaviersanchez many thanks for bearing with me. I can merge this. The remaining question is whether we need to add some sort of validation data for this test?

@fjaviersanchez
Copy link
Contributor Author

Thank you @yymao. I would say so. Let me please check for a good sample or model.

@fjaviersanchez
Copy link
Contributor Author

@yymao I think that this is PhoSim's cleanest run to get a good estimation of the power spectra: /global/projecta/projectdirs/lsst/production/DC2/DC2-phoSim-test_WFD-r/output/000035. What would be the most effective strategy to generate the validation data? Run the script that we have now, dump the output somewhere and re-use it or read these images everytime?

@yymao
Copy link
Member

yymao commented Aug 28, 2018

@fjaviersanchez that could work. What's the precision that you'd like the PSD to be validated? We could also fit a functional form and use the fitted PSD to validate?

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.

@fjaviersanchez thanks for the commits. I made one comment about the score calculation. Also, I just ran this PR and think the validation data do not look right. Can you take a look? You can find the DESCQA run here

if count:
total_chi2 /= count
ndof = len(self.validation_data['k']) - 1
score = 1 - chi2.cdf(total_chi2, ndof)
Copy link
Member

Choose a reason for hiding this comment

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

Probably no 1 - here, since the passing criterion is passed=(score < 0.95)?

score = chi2.cdf(total_chi2, ndof)

@fjaviersanchez
Copy link
Contributor Author

@yymao the new validation data should be more representative of most of the images that we'll analyze since I am including sources (the previous one had no sources in the image, that's why it had very low power). I also corrected the bug with the score. Thanks!

@yymao
Copy link
Member

yymao commented Aug 29, 2018

Thanks @fjaviersanchez! This looks great. Here's latest DESCQA run. I'm merging this now.

@yymao yymao merged commit d65d1c3 into LSSTDESC:master Aug 29, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants