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

Adding members_only option for redmapper catalogs #317

Merged
merged 4 commits into from
May 31, 2019

Conversation

yymao
Copy link
Member

@yymao yymao commented May 31, 2019

A redmapper catalog contains both members information and clusters information, and obviously they would have different number of rows (unless every cluster has only one member, but we don't live in that universe).

GCR by itself does not requires all columns have the same number of rows, and hence the redmapper reader work fine as it. A column of "members" table just have different number of rows from a column of "clusters" table.

However, when matching the redmapper catalog to the underlying protoDC2 catalog, the matched catalog can have only one single table, and in this case, it's the members table that is needed (i.e., each "member", not "cluster", in the redmapper catalog corresponds to a galaxy in the protoDC2 catalog).

Hence, this PR introduces a members_only option so that when the redmapper catalog is used as an "add-on", only the members part is matched to the protoDC2 catalog.

This PR fixes #315.

@yymao yymao added the composite Issues related to composite catalogs label May 31, 2019
@yymao yymao requested review from erykoff and EiffL May 31, 2019 19:30
@yymao yymao added this to the v0.11.0 milestone May 31, 2019
@erykoff
Copy link
Contributor

erykoff commented May 31, 2019

I guess this PR doesn't cover the matching itself? Because one galaxy in the CosmoDC2 table can show up multiple times in the members catalog (just so long as the total membership probability is <=1), and I think that this PR doesn't break that relationship.

@yymao
Copy link
Member Author

yymao commented May 31, 2019

This PR is just removing the "clusters" table when doing the matching. The matching itself is not touched.

However, the matching is in fact a left join, and right now protoDC2 is on the left. But according to what you said it seems that we should have put redmapper on the left? @erykoff

@erykoff
Copy link
Contributor

erykoff commented May 31, 2019

I believe that is correct, that the redmapper members should be on the left.

erykoff
erykoff previously approved these changes May 31, 2019
@yymao
Copy link
Member Author

yymao commented May 31, 2019

@erykoff thanks. Just made the change (moving redmapper catalog to the left for the composite catalog). Take a look?

@erykoff
Copy link
Contributor

erykoff commented May 31, 2019

I guess the fact that it's a left join and which catalog is on the left is documented elsewhere in the code?

@yymao
Copy link
Member Author

yymao commented May 31, 2019

@yymao yymao merged commit b1b11db into master May 31, 2019
@yymao yymao deleted the u/yymao/redmapper-fix branch May 31, 2019 21:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
composite Issues related to composite catalogs
Projects
None yet
Development

Successfully merging this pull request may close these issues.

protoDC2 redmapper add-on catalogs do not work
2 participants