Added capability to save Gaussian skewers

[jfarr03]

As per issue #34, where the possibility of adding the capability to save skewers of the Gaussian field rather than the physical, lognormal field was discussed.

This is implemented by a flag in the param.cfg file 'gauss_skewers' which causes the Gaussian field values to be recalculated on the fly during interpolation in the production of the skewers.

When the flag is turned off, the exact same results as before are obtained. Below are two plots of the mean and variance in the density skewers as a function of rest frame wavelength. These are for:

1. The original code:

2. The modified code:

When the flag is turned on, the Gaussian skewers saved now have zero mean far away from the quasars, whereas the original skewers (calculated using an inverse lognormal transformation on the physical density skewers) did not (this is the problem described in #34):

1. Original Gaussian skewers (recalculated from physical skewers):

2. New Gaussian skewers (outputted by CoLoRe):

Indeed, individual new Gaussian skewers show the exact same features as the old ones, but are shifted to a different mean value:

As far as I can tell, it seems to be working as required, but let me know if there are any things which could be done better!

[andreufont]

Looks good, thanks for the plots!

@damonge - let us know if there is anything you'd like us to change from the point of view of CoLoRe structure.

[andreufont]

@damonge - is there anything else we should do before merging?

[damonge]

Sorry, let me have some time to look at this before merging! Definitely ping me again on Monday if it hasn't happened.

[andreufont]

There is no rush, I just had forgot about it as well. We'll ping you again next week, enjoy the meeting :-) Andreu

On 18 April 2018 at 18:35, David Alonso notifications@github.com wrote:

Sorry, let me have some time to look at this before merging! Definitely ping me again on Monday if it hasn't happened.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/damonge/CoLoRe/pull/36#issuecomment-382468188, or mute the thread https://github.com/notifications/unsubscribe-auth/ADscR1iNNOdBqVX0zxFkK5kzc08V7fOwks5tp3lwgaJpZM4Seh2g .

[damonge]

Oh, just saw I've been ignoring this since more than a month. Sorry about that, I'll look into this ASAP!

[andreufont]

Hi @jfarr03 - is this the branch that you have been using for your mocks? If so, should we try to merge it to master?

[jfarr03]

Yes this is the one! One thing that I didn't think of at the time, though, was what happens if someone asks for non-lognormal density to be used (LPT/2LPT), but also to return the Gaussian skewers. In theory it should be fine but I haven't tested it at all.

[damonge]

Thanks @andreufont @jfarr03 . We can merge this one if you want to, but do we want to address. James' question first? Looking at line 55 in beaming.c, I think the modifications are only valid for lognormal fields.

[damonge]

(to clarify: happy to merge as is, but just pointing this out)

[andreufont]

Hi @damonge - we can chat now on Skype, but I think for now we should just raise an error and exit if you are asking for Gaussian skewers but running 2LPT simulations.

[jfarr03]

Yes I think raising an error and exiting would be the most straightforward thing for now. What would be the best way to do this? I see a function report_error in common.c, which might be an option?

[damonge]

Yes, go for that

[andreufont]

Hi @jfarr03 - any progress on this?

[jfarr03]

Yes - I put in a fix and sent test jobs running. I’ll check them later tonight to make sure it’s worked and then I’ll push to the branch.

[jfarr03]

@damonge I've now added 2 lines to raise an error if Gaussian skewers are requested from a non-lognormal box.

As part of this PR, I've also added the option to the example param.cfg file, but this has now been moved to a different location. I don't know if git can deal with this, but it might be easiest for me to remove that option again, then you can merge the PR, and finally I'll submit another, small PR re-adding the option.

Hopefully that makes sense - let me know if not, or if there's a simpler option!

[damonge]

Thanks a lot @jfarr03 What you propose sounds good. Before doing anything, let me track down a possible memory leak in the current master, in case there's something else in your code that needs modifying. I'll try to get that done shortly.

[jfarr03]

Sure that sounds good - I'll remove the changes to the param file now, and then you can merge whenever you've tracked down the leak!

[damonge]

OK, let's actually merge this one first. Thanks a lot @jfarr03 , @andreufont !