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

Simplify tomography files and mappers #317

Merged
merged 6 commits into from
Aug 24, 2023
Merged

Simplify tomography files and mappers #317

merged 6 commits into from
Aug 24, 2023

Conversation

joezuntz
Copy link
Collaborator

@joezuntz joezuntz commented Aug 1, 2023

Tomography files unnecessarily distinguish "source_bin" and "lens_bin" columns in files, making it harder to use them generically.

This changes this so that all tomo files have a set of common columns. At the same time, I removed the old TXMainMaps class, which assumed identical lens and source catalogs, because it would not work straightforwardly with this change (because both lens and source bins are now called "bin"), and rewrote the Mapper class as ShearMapper and LensMapper (it was never being used to do both at once outside TXMainMaps).

As a side point this finally does the source number count maps.

This was making the code more complicated than it needed to be when writing code to use QP and do truth photo-z summations.

@joezuntz
Copy link
Collaborator Author

@empEvil did you have a chance to look at this?

@empEvil
Copy link
Contributor

empEvil commented Aug 16, 2023

No not yet, I happened to get sick :(

@joezuntz
Copy link
Collaborator Author

Get well soon!

Copy link
Contributor

@empEvil empEvil left a comment

Choose a reason for hiding this comment

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

Think it looks good. Are there any places the code could get confused now that the names have been removed? (lens and source)

txpipe/mapping/basic_maps.py Outdated Show resolved Hide resolved
txpipe/noise_maps.py Show resolved Hide resolved
txpipe/noise_maps.py Show resolved Hide resolved
@joezuntz
Copy link
Collaborator Author

Thanks for the review! As far as I can tell I've dealt with all the places where there's a name confusion. I'm sure we'll hit something else eventually!

@joezuntz
Copy link
Collaborator Author

Could you approve if you're happy?

@joezuntz joezuntz merged commit a5f83ad into master Aug 24, 2023
10 checks passed
@joezuntz
Copy link
Collaborator Author

Thanks!!!

@joezuntz joezuntz deleted the simplify-tomo-files branch August 24, 2023 16:17
combet added a commit that referenced this pull request Sep 7, 2023
…in master from PR #317 (simplify tomography files and mappers)
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.

2 participants