igmhub / cup1d

Cosmology using P1D - small scale clustering of the Lyman alpha forest
2 stars 1 forks source link

Generate random realizations of P1D measurements according to cov matrix #55

Closed jchavesmontero closed 9 months ago

jchavesmontero commented 9 months ago

This PR addresses #24.

andreufont commented 9 months ago

Let's merge #54 first, update this and review.

However, I would like to chat about this in person (are you at IFAE?), since this is not quite what I had imagined.

andreufont commented 9 months ago

Let's have the chat here. I was imagining that the option to add noise would be passed in the construction of the P1D measurement, and that the perturbed P1D would be stored as if it was a normal P1D. That way you make sure that any function that interacts with the P1D object will always get the perturbed P1D, for plotting, for the likelihood, etc.

I thought I would add this to one of the mock_p1d objects, and not in the base_p1d class, but I guess it doesn't hurt to have it there, and this way you can recycle the code for the different p1d objects that are mock data. The proper "object-oriented" way to do this would be to have a base_mock_p1d class that inherits base_p1d and adds this functionality, and then objects like eboss_mock or the older mock objects would inherit base_mock_p1d, but I have been known in the past for going too deep into object-orienting programing, so I will not insist :-)

jchavesmontero commented 9 months ago

In the last commit, I added the base_p1d_mock.py file and I modified the data_eBOSS_mock.py file to inherit this new class. @andreufont I think I did what you wanted, but could you please take a look before I modify other files. You can see how the new eboss_mock works in the new version of plot_add_noise_to_mock_data.ipynb.

jchavesmontero commented 9 months ago

@jchavesmontero - this looks great! I added two minor comments (one file can be deleted, and one argument is not necessary), but happy to merge otherwise.

We should modify a couple of other objects (data_gadget, data_nyx, data_mock) so that they also inherit base_p1d_mock instead of base_p1d_data. This would allow us to do noisy mocks of these as well.

Sure! I just wanted to be sure that everything was looking good before doing so. Working on this now