Skip to content

Adding spectroscopic stretch (FoG model)#603

Open
iagolops wants to merge 6 commits into
LSSTDESC:masterfrom
iagolops:master
Open

Adding spectroscopic stretch (FoG model)#603
iagolops wants to merge 6 commits into
LSSTDESC:masterfrom
iagolops:master

Conversation

@iagolops

@iagolops iagolops commented Jan 9, 2026

Copy link
Copy Markdown

Description

This commit implements a new systematic, SpecZStretch, which convolves the $N(z)$ distribution with a window chosen to model the Fingers-of-God effect. The theoretical approach for this modelling is discussed in 2008.08706. In this commit, we've added the Gaussian and the Lorentzian windows, where the systematic parameter is the velocity dispersion ($\sigma_v$) within the redshift bin, where its unity is $\text{km}/\text{s}$.

The (unnormalized) Gaussian convolution is given by:

$$ n_{conv}^{i}(z) = \int_{-\infty}^{\infty} n^i(s) \exp\left(-\frac{(z-s)^2}{2\sigma_{s,i}^2}\right) ds. $$

Here, $\sigma_{s,i}$ is the redshift dispersion in the $i$-th redshift bin, which is related to the velocity dispersion by:

$$ \sigma_{s,i} = (1+ z_{mean,i})\frac{\sigma_{v,i}}{c}. $$

The image below shows the result of this implementation. It shows the behavior of cross-correlations when applying this new systematic into spectroscopic $N(z)$ distributions.

cell_vs_sigma_v_rsd

We (I, @arthurmloureiro, and Bruno Moraes) implemented this systematic based on PhotoZShiftandStretch, where we added the two main functions dndz_stretch_fog_gaussian and dndz_stretch_fog_lorentzian, which are similar to dndz_shift_and_stretch_active.

To-do:

  • Add tests
  • Check how the user can change the kernel from Gaussian to Lorentzian in the __init__ of SourceGalaxySpecZStretch

Type of change

  • New feature (non-breaking change which adds functionality)

Checklist:

  • I have run bash pre-commit-check and fixed any issues
  • I have added tests that prove my fix is effective or that my feature works
  • I have made corresponding changes to the documentation
  • I have 100% test coverage for my changes (please check this after the CI system has verified the coverage)

@arthurmloureiro

Copy link
Copy Markdown
Member

@LSSTDESC/firecrown-dev : @iagolops and I had a really awful time tracking down the new dependency trees in firecrown 1.15. At some point we had more than 6 different files to try to track down where the code for PhotoZShiftandStretchFactory was so we could mimic the same for this case and we could not do it.

There's no symmetry between firecrown.likelihood.weak_lensing folders/imports/file structure and firecrown.likelihood.number_counts and seems like the bulk of the code has been transferred to this _base.py while the rest is just an endless maze of class inheritances and imports.

I think the experience now from trying to implement this systematic factory and the CMB Lensing factory is that firecrown is becoming really really hard to understand and develop for without help from the core developers. It's a maze. 😵‍💫

@marcpaterno

Copy link
Copy Markdown
Collaborator

Hi @arthurmloureiro, we're sorry for the current messy state. As we have noted in previous CCL/Firecrown meetings, what you're observing is the ongoing process of our reorganization of the physical structure of the package. The messy likelihood module is the main thing that has not yet been reorganized.

@codecov

codecov Bot commented Jan 13, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 26.22951% with 45 lines in your changes missing coverage. Please review.
✅ Project coverage is 99.5%. Comparing base (a92b250) to head (2919d2c).

Files with missing lines Patch % Lines
firecrown/likelihood/_base.py 22.4% 45 Missing ⚠️

❌ Your patch check has failed because the patch coverage (26.2%) is below the target coverage (100.0%). You can increase the patch coverage or adjust the target coverage.

Additional details and impacted files
@@           Coverage Diff            @@
##           master    #603     +/-   ##
========================================
- Coverage   100.0%   99.5%   -0.5%     
========================================
  Files         151     151             
  Lines        9035    9096     +61     
  Branches     1033    1038      +5     
========================================
+ Hits         9035    9051     +16     
- Misses          0      45     +45     
Files with missing lines Coverage Δ
firecrown/likelihood/_source.py 100.0% <ø> (ø)
firecrown/likelihood/number_counts/__init__.py 100.0% <100.0%> (ø)
firecrown/likelihood/number_counts/_factories.py 100.0% <100.0%> (ø)
firecrown/likelihood/number_counts/_systematics.py 100.0% <100.0%> (ø)
firecrown/likelihood/_base.py 88.5% <22.4%> (-11.5%) ⬇️
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@arthurmloureiro

Copy link
Copy Markdown
Member

Hi @arthurmloureiro, we're sorry for the current messy state. As we have noted in previous CCL/Firecrown meetings, what you're observing is the ongoing process of our reorganization of the physical structure of the package. The messy likelihood module is the main thing that has not yet been reorganized.

Hi @marcpaterno !

Thanks for letting us know that this is not the final stage of this refactoring.

If I can voice a few concerns about the refactorings in Firecrown:

  1. Please tell me that this labyrinth of imports is something temporary. I feel like there was a jump in orders of magnitude in the abstraction in firecrown. It's ok if this is is just a temporary structure while things are being changed, but if this is close to the final structure, it will be impossible for others to understand the code and develop for it. It's ever more abstract than it was before. I consider myself quite familiar with firecrown and I do not understand anything anymore.
  2. Over the past few months, we have an increasing number of projects and pipelines that are dependent on firecrown. Although some more official pipelines are considered when structure changes, the other projects have a severe amount of overhead to keep up with the pace of changes in the import trees and function calls. This is causing several delays and impacting a lot of student's whose projects have very tight deadlines.

About 2) my suggestion would be to not deprecate structure between small version changes or we should instead avoid having any student projects to use firecrown until a stable and immutable structure is chosen. First one would be best, given that there currently several student projects ongoing that are using firecrown.

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.

4 participants