Skip to content

Conversation

@martinkilbinger
Copy link
Contributor

@martinkilbinger martinkilbinger commented Jan 20, 2026

Running sp_validation on MCCD v1.3.6 catalogue.

@cailmdaley
Copy link
Collaborator

Code review

Found 2 issues:

  1. Hardcoded absolute path with username in mask field - The new SP_v1.3.6 config uses /home/guerrini/sp_validation/... which won't work for other users. Other path references in this file use relative paths or the subdir pattern.

n_psf: 0.752316232272063
sigma_e: 0.379587601488189
mask: /home/guerrini/sp_validation/cosmo_inference/data/mask/mask_map_v1.4.6_nside_8192.fits
psf:
PSF_flag: FLAG_PSF_HSM

  1. Dependencies removed without explanation - Four dependencies were removed (cosmo-numba, cs_util, pymaster, shear_psf_leakage) that are used in multiple files across the codebase. The PR description doesn't mention why these are being removed, which could break existing functionality.

"camb",
"clmm",
"colorama",
"emcee",
"getdist @ git+https://github.com/benabed/getdist.git@upper_triangle_whisker",
"h5py",
"healpy",
"healsparse",
"importlib_metadata",
"joblib>=0.13",
"jupyter",
"jupyterlab",
"jupytext==1.15.1",
"lenspack",
"lmfit",
"numexpr",
"numpy>=1.14",
"opencv-python-headless",
"pyccl",
"pyarrow",
"regions",
"reproject",
"scipy",
"seaborn",
"skyproj",
"statsmodels",
"treecorr>=5.0",
"tqdm",
"uncertainties"
]
# "cosmo-numba @ git+https://github.com/aguinot/cosmo-numba",


Generated with Claude Code

If this code review was useful, please react with 👍. Otherwise, react with 👎.

@martinkilbinger
Copy link
Contributor Author

I agree, the absolute path is not great, but readable for other users (it works for me).
I think I removed the cosmo-numba dependence, because it requires python 3.12. I am about to create a new docker file with 3.12, we can merge this first to get cosmo-numba back.

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