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

redmapper conditional luminosity function test #102

Merged
merged 46 commits into from
Nov 29, 2023
Merged

Conversation

chto
Copy link

@chto chto commented Apr 10, 2018

I have implemented a conditional luminosity function measurement on redmapper catalog. [as discussed in #9 ] The run on protoDC2-v2 redmapper catalog looks like
clf_redmapper
[I somehow got permission denied to the DESCQA web directory, so I cannot post the link ].

Pining @j-dr @yymao @erykoff as interest parties.

[Edited by @yymao: This PR fixes #9 ]

@yymao
Copy link
Member

yymao commented Apr 10, 2018

@chto thanks. I haven't looked at the code in detail but wonder if this test can be consolidated with the existing code in descqa/clf_test.py?

@chto
Copy link
Author

chto commented Apr 10, 2018

@yymao I am not sure. descqa/clf_test.py mainly deals with halo catalog, while this one deals with redmapper catalog, which requires consideration of limiting magnitude, membership probability, etc.
I think this is a different level of test on simulations, so it's probably better to keep them separate.

@yymao
Copy link
Member

yymao commented Apr 10, 2018

@chto hmm, ok. I'll have to take a closer look at the code to be sure. My thought was that test can use different sources (true halos or redmapper groups) as halos. In the case of true halos, the membership probabilities are just 1 and 0.

Anyway, I'll come back to this tomorrow.

@yymao yymao requested a review from j-dr April 10, 2018 15:55
@yymao
Copy link
Member

yymao commented Apr 10, 2018

@chto also, can you clean up the code a bit. In particular,

  • removing testing statements like print('test2')
  • enforcing a consistent coding style (e.g., have spaces before and after operators)

Also, there's a large number of for i in range(...) statements. In most cases these can be avoided (either by directly looping over the iterable itself, or by using numpy functions). If it's helpful, I can help rewrite some of these to give you an example.

@chto
Copy link
Author

chto commented Apr 10, 2018

@yymao Sure. I will try to make this more readable shortly.

@chto
Copy link
Author

chto commented Apr 16, 2018

@yymao I tried to make the code more readable and tried to avoid "for i in range" statement. But, there are still places where I need the index of the array, so I cannot avoid that statement completely.

@yymao
Copy link
Member

yymao commented Apr 16, 2018

@chto thanks. I'm traveling this week but will try to find time taking a look at your updates.

@chto
Copy link
Author

chto commented Apr 21, 2018

@yymao @j-dr I add the clf measurement on sdss as a validation data.
I abundance matching the dc2 halo to sdss in richness bins 5-10, 10-15, 15-100.
I found that since dc2's area is too small and sdss only go out to z=0.3, the information is not enough to make a good comparison.
One possible solution is to use des y1 as the validation data, but I am not sure whether that dataset is public or not.
clf_dc2

@chto
Copy link
Author

chto commented Dec 11, 2020

@evevkovacs,
I have implemented @rmjarvis' suggestion with small modification.
I have run the code and it runs!

However, I notice that /global/common/software/lsst/common/miniconda/current/envs/desc/bin/python doesn't have kcorrect_python installed, which is needed for the module and can be installed by pip.

@heather999
Copy link

heather999 commented Dec 11, 2020

@rmjarvis @chto Is this the correct kcorrect_python, that's the webpage linked form pypi? My attempt to do a pip install failed, using the tarball and just doing a pip install kcorrect_python. I can go back and try a manual install as the README suggests. Do we have some confidence about the long term support of this package?

@erykoff
Copy link

erykoff commented Dec 11, 2020

Oof. The only kcorrect_python that I can find (same as you linked to) is python 2 only. So that's not going to work with our software.

@evevkovacs
Copy link
Contributor

Oh dear. This doesn't seem like a good solution after all. Should we go back to Erin Sheldon's version https://github.com/esheldon/kmeans_radec

@rmjarvis
Copy link

kcorrect_python has nothing to do with the TreeCorr kmeans. This is an orthogonal issue.

@rmjarvis
Copy link

The last commit to kcorrect_python was over three years ago. I'd probably recommend not using that and finding some other package that can perform the same (or equivalent) calculation. Maybe something in astropy?

@erykoff
Copy link

erykoff commented Dec 11, 2020

There is nothing in astropy that does "kcorrections" (TM). Depends on what functionality we're using on how hard it is to put together a replacement.

@evevkovacs
Copy link
Contributor

Sorry I misunderstood the nature kcorrect_python issue.

@rmjarvis
Copy link

Yeah, I found some discussions about making an affiliated package for kcorrect, but it doesn't seem to have gone anywhere. What I did find was the astrophysics package, which claims to implement the Blanton k correction:
https://pythonhosted.org/Astropysics/coremods/phot.html?highlight=kcorrect#astropysics.phot.kcorrect
Furthermore, I think this project is being considered for incorporation into astropy, so that's probably a long term workable option if this does what you need.

@rmjarvis
Copy link

Ugh. Never mind. That project also seems to be defunct now. :(

@rmjarvis
Copy link

I can't find anything that does this as part of a maintained python package.

OTOH, the license for kcorrect_python is listed as "Public Domain", so you could just copy the relevant parts of that code into this repo, make them work for python 3, and then just use that going forward. That would be better IMO than depending on an unmaintained package.

@evevkovacs
Copy link
Contributor

evevkovacs commented Dec 11, 2020

@chto. How critical is to run this kcorrect_python for the test? Could it be dropped? Alternatively, could it be applied one time to the validation data, so that we don't need to run it each time for the test? I am not sure how the module is used, so I am just throwing out ideas for how to get around the issue.
Or see mike's suggestion above. Thanks!

@yymao
Copy link
Member

yymao commented Dec 12, 2020

Want to echo @evevkovacs's comment above --- When I was cleaning up the test code in this PR, I already made kcorrect an optional dependency. It is used only when the k-corrected validation data set does not exist (see here and here and ). So the k-correction can be run once (outside DESC environment) and the resulting validation data can be checked in to data directory of this repo.

If approximated k-correction is acceptable, you can also consider using http://kcor.sai.msu.ru/calc_kcor.py --- This is not a package but a single python function that contains fitting functions for approximated k-correction. The source is licensed under BSD so we could include it in our source with proper attribution.

@chto
Copy link
Author

chto commented Dec 12, 2020

@yymao I think it is the other way around. The k-correction is done on the simulated catalog in the same way as we do on the comparison data. I think maybe we can remove k-correction on both side but there will be an issue if the filters used in the simulation are different from the comparison data (SDSS).

Further, I don't think the approximated k-correction mentioned above can handle arbitrary filters.

Here is my proposal:

  1. We add a comparison data without k-correction applied.
  2. If the user has kcorrect-python installed, we do the comparison as it is. If not, we can compare the non-kcorrected simulation luminosity to non-kcorrected data luminosity, and we print a warning message.

@yymao
Copy link
Member

yymao commented Dec 12, 2020

@chto --- thanks, that sounds like a good solution to me. If you can help implement it that'd be great.

@chto
Copy link
Author

chto commented Dec 12, 2020

@yymao Sure. Working on it.

@evevkovacs
Copy link
Contributor

@chto @yymao I took a quick look in the code and noticed that "possible_mag_fields" includes only "mag_{}lsst". (You are trying to compare to SDSS data, and that is why you are doing k-corrections, right?) But did you know that cosmoDC2 has SDSS magnitudes already available ('mag{}sdss')? I would suggest prepending 'mag{}_sdss' to the list of possible_mag_fields and if they are available, also skipping the call to kcorrect. This would be a minor modification to step 1, above. There are also fits to k-corrections between various filters in CheckColors.py derived by Nacho Sevilla.
The ones for lsst filters are given in lines 40-56. You could use these for a transformation between lsst and sdss

@chto
Copy link
Author

chto commented Dec 14, 2020

@yymao @evevkovacs I have implemented suggestions above and the code runs on proto-dc2_v3.0_redmapper. However, I also notice that it cannot be run on cosmoDC2_v1.1.4_redmapper_v0.5.7, because the naming conventions are different and also miss a required field, lim_limmag_dered.

Also, there is no SDSS magnitudes in cosmoDC2_v1.1.4_redmapper_v0.5.7.

@yymao
Copy link
Member

yymao commented Dec 14, 2020

Thanks @chto! And I wonder if @erykoff can answer your questions.

@evevkovacs
Copy link
Contributor

evevkovacs commented Dec 14, 2020

@chto So you could add a variable like possible_redmapper_quantities and list the allowed values that work for both redmapper catalogs, and then use the grc function "first_available" to get whichever one is given in the version of the catalog is being used. Please ask @erykoff about lim_limmag_dered; I would think that there is a substitute or work-around for this quantity. For the missing SDSS magnitudes, why not use the conversion from lsst to sdss filters that I pointed you to in CheckColors.py . Thanks

@chto
Copy link
Author

chto commented Dec 15, 2020

@evevkovacs
Thanks for the reply. I guess I am not entirely sure what is the newest catalog which you want to test. (or alternatively, what are the catalogs that you want to test?)
I was assuming it is cosmoDC2_v1.1.4_redmapper_v0.5.7, but this catalog doesn't have the mag_{}_sdss you mentioned above. So, I am wondering this might not be the one you mentioned before.

@evevkovacs
Copy link
Contributor

@chto Sorry I wasn't clear. I wanted to run on cosmoDC2_v1.1.4_redmapper_v0.5.7 and was suggesting that you use the lsst to sdss conversion in CheckColors.py to get around the problem of not having mag_{}_sdss available.

@yymao yymao merged commit 5a884e4 into LSSTDESC:master Nov 29, 2023
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.

testing luminosity function of cluster galaxies (satellites, centrals)
7 participants