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

few fixes #376

Open
wants to merge 16 commits into
base: master
Choose a base branch
from
Open

few fixes #376

wants to merge 16 commits into from

Conversation

marina-ricci
Copy link
Contributor

  • we still need to figure the coordinate problem
  • the CLMM version for installed is not updated apparently
  • we need to clean the repo
  • merge with Tomomi's working branch
  • understand how to use true shear
  • understand how to use full cosmoDC2 datasets

@eduardojsbarroso
Copy link

Hi @marina-ricci, I just added the code to convert the output in notebooks/cluster_counts/Run_CL_pipeline.ipynb to the SACC format. However I do not have permission to make a push here. Can you or @joezuntz add me as a contributor to TXPipe?

@joezuntz
Copy link
Collaborator

I've added you to the TX team, so you should have access now @eduardojsbarroso

@eduardojsbarroso
Copy link

@marina-ricci However I have two points on the code I just pushed:

  1. The cluster ensemble object does not seem to have the radius binning. I did not find the collumn radius_min and radius_max that is usually in the cluster individual profiles. Is this information missing or I just don't know how to access? Since I did not have it, I invented some radius_binning. I need to change that
  2. The output TXPipe file does not have the area of the survey in it. Is this something we intend to add to the catalog?

@eduardojsbarroso
Copy link

@marina-ricci I took the code I had on the notebook and created a convert_to_sacc stage and added this stage to the scripts, so now we can run the pipeline and have the sacc file at the end.

Is there other parts of this pull request I can work on so we can merge?

@joezuntz
Copy link
Collaborator

The failing test was just because we were testing TXTwoPointRLens in the redmagic pipeline file. I've removed it since it wasn't really needed there.

Copy link
Collaborator

@joezuntz joezuntz left a comment

Choose a reason for hiding this comment

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

I'm pre-approving this so CL people can merge if they want to, since it only affects the extension module.

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.

3 participants