Skip to content

Conversation

itrharrison
Copy link
Collaborator

@itrharrison itrharrison commented Jul 4, 2025

I am creating this partially for me own interests / forecasting purposes(!), but it is useful to have here. Including to keep in line with the smoothTheory example.

@itrharrison itrharrison self-assigned this Jul 4, 2025
@itrharrison itrharrison added enhancement New feature or request crosscorrelation Related to the CrossCorrelation likelihood labels Jul 4, 2025
@itrharrison itrharrison marked this pull request as draft July 4, 2025 13:57
@ggalloni
Copy link
Collaborator

ggalloni commented Sep 5, 2025

Hi @itrharrison, given that this is still a draft, this comment is probably premature, but let me write it here nonetheless.
I was having a look at the cross_correlation module with the new ShearShearLikelihood. To have the least duplication possible, we could:

  • Rename the current CrossCorrelationLikelihood into something like _BaseCorrelationLikelihood, which should deal with both auto and cross correlations through a flag like is_cross = True or is_cross = False. I am thinking, for example, of the step checking for the correctness of the tracer.
  • At this point, CrossCorrelationLikelihood and a new AutoCorrelationLikelihood can just be defined by inheriting the base class and fixing the flag to True and False, respectively.
  • Now everything can stay as it is, except ShearShearLikelihood that should inherit the autocorrelation.
  • This will make the module more scalable than it is right now, in the sense that if we want to add other autocorrelations, we don't need to duplicate code further. On a side note, I think _get_ia_bias should be part of the base class so that it can be reused to avoid some duplication present in the current module.

In any case, if the ShearShear stays there one way or the other, we should probably rename the module itself, since cross_correlation is not really correct. The first idea that comes to mind is just to drop "cross", but I am not convinced. Maybe we could even split the two into two modules, I don't know. Did you already think about this?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
crosscorrelation Related to the CrossCorrelation likelihood enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants