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

apparent magnitude function test #47

Merged
merged 126 commits into from
Jun 11, 2018
Merged

apparent magnitude function test #47

merged 126 commits into from
Jun 11, 2018

Conversation

duncandc
Copy link
Contributor

@duncandc duncandc commented Dec 8, 2017

This is a premature pull request for the apparent magnitude function test.

@duncandc
Copy link
Contributor Author

duncandc commented Jun 6, 2018

I am fiddling with it right now, but it currently works and can be run following @yymao 's directions.

@evevkovacs
Copy link
Contributor

@duncandc @yymao @aphearin Thanks very much, the results are here Although, formally, we fail the tests, visual inspection of the plots leads me to conclude that the agreement is acceptable (and meets the target of falling below the requirement in anticipation of the increase in dn/dmag due to the increase in the redshift range for cosmoDC2 )

@duncandc
Copy link
Contributor Author

duncandc commented Jun 6, 2018

Thanks for running that. It looks like the bottom difference panel is not showing what it should. I will fix that.

@yymao
Copy link
Member

yymao commented Jun 6, 2018

@evevkovacs thanks! The catalog seems be to doing pretty good.

I believe we discussed about loosening criteria; maybe @duncandc is still working on that? Another aesthetic suggestion for @duncandc: making the grey band lighter/more transparent.

@duncandc
Copy link
Contributor Author

duncandc commented Jun 6, 2018

The criteria has been loosened, but it is easily changed to whatever we want in the test by changing the tolerance or magnitude range arguments.

@evevkovacs
Copy link
Contributor

I think the catalog is doing very well, but wait for @aphearin to have the final word.

@yymao
Copy link
Member

yymao commented Jun 6, 2018

@duncandc what's the current criteria?

I think @rmandelb's earlier suggestion was "20%-level match for 23 < i < 25.3, and 40%-level match for 25.3 < i < 27". But given what we've learned, maybe we should be between 0 and -50% for 23 < i < 26.5?

(cc @aphearin)

@aphearin
Copy link

aphearin commented Jun 6, 2018

This level of agreement is what I anticipated based on the calibration experiments appearing earlier in this thread. The turnover at r>26 is expected, as we decided to postpone calibrating model components that will rectify this until we have cosmoDC2 cutouts to work with, as discussed with @rmandelb and @janewman-pitt-edu.

@duncandc
Copy link
Contributor Author

duncandc commented Jun 6, 2018

The current tests are for 40% agreement to the extrapolated HSC measurements between 24.0 and 27.5 for all 5-bands, but it is completely trivial to set whatever requirements you like.

@duncandc
Copy link
Contributor Author

duncandc commented Jun 6, 2018

also, just for reference, here is a diagnostic showing the HSC 'raw results', power law fits, and the final 'extrapolated' version used for these tests.

app_mag_func

@evevkovacs
Copy link
Contributor

@duncandc @yymao We are planning to have a final version of protoDC2 v4.6 ready in the next day.
Is there any possibility that the dn/dmag test could be in the master branch by late tomorrow so we could do a full descqa run of all the required validation tests? Otherwise let me know if there are updates to that development branch that I will need to pull. Thanks Eve

@duncandc
Copy link
Contributor Author

duncandc commented Jun 6, 2018

sure, I will put in a pull request this afternoon.

@duncandc
Copy link
Contributor Author

duncandc commented Jun 7, 2018

ok @yymao this should be ready to go. You can see an example run here: https://portal.nersc.gov/project/lsst/descqa/v2/?run=2018-06-07_6

@aphearin let me know if you want something changed.

@evevkovacs
Copy link
Contributor

@duncandc @yymao @dkorytov @danielsf I was curious as to why the y-band was being skipped. Dan looked into it. In the reader, the quantity is mag_true_Y_lsst, not mag_true_y_lsst (see line 260 in alphaq.py) (and similarly for all the other Mag quantities). It looks like this might be an issue in buzzard too, since it is also skipped. Which of these is actually correct? Is it y or Y? Did we spell this out in the schema? Thanks.

@yymao
Copy link
Member

yymao commented Jun 7, 2018

We should use lower case y, but we didn't know that when we design the schema. We should submit an issue to GCRCatalog to fix this.

@evevkovacs
Copy link
Contributor

@duncandc @yymao If I want to run the latest version of the test, how do I get the code? I tried a git pull on the cloned branch that I made yesterday, but it said I was up to date. Thanks.

@yymao
Copy link
Member

yymao commented Jun 7, 2018

@evevkovacs

git checkout duncandc-devel
git pull https://github.com/duncandc/descqa.git devel

@yymao
Copy link
Member

yymao commented Jun 8, 2018

@aphearin @rmandelb I wonder if you have further comments on this test? If not, I'd like to merge this.

@yymao yymao merged commit fd591c3 into LSSTDESC:master Jun 11, 2018
@aphearin
Copy link

For posterity's sake, I'm posting an updated plot showing <Mr|M*, z> in cosmoDC2 v1.0.0. This is to be contrasted with the protoDC2 result that I posted earlier in the comment above. My earlier plot showed that protoDC2 had an unphysically strong redshift evolution at the faint end, revealed to be grossly inconsistent with the COSMOS data shared by @rmandelb that appears in this comment of this thread. Modeling <Mr|M*, z> with reasonable accuracy turned out to be mission-critical for simultaneously fitting HSC dn/dmag and DEEP2 dn/dz. So thanks again to everyone here for the hard work in nailing down these validation criteria.

mr_mstar_evolution

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.

None yet

10 participants