Lens Constructors

[rbmetcalf]

Modified constructors of Lens so that internal values are set correctly when there is no parameter file.

[rbmetcalf]

@FabienNugier see if this works for you.

[FabienNugier]

The code still has the problem that you understood yesterday. Namely that the positions of the halos are zero and thus the tree keeps on searching. I found the reason why this happens. Indeed in LensHalos the function getX differs from getTheta by a distance called Dist and this one is apparently not set in these conditions. I am going to look deeper because it should be set through insertMainHalo.

[rbmetcalf]

I have a feeling you are trying to do something the LensHalo was not intended for.

[rbmetcalf]

The Dist in LensHalo was something of a compromise. Many of the derived classes don't need to know what their redshift is, but because of legacy some do such as the NSIE.

Anyway, I put in some crosschecks that I think should help locate the problem and make it more obvious if it happens again.

[rbmetcalf]

This might fix the problem with constructing from a parameter file.

Please merge only from the master. Do not merge between development branches. This will make it much more difficult to track down bugs.

Wait! I've made a small mistake.

[rbmetcalf]

OK. That should have fixed it.

[FabienNugier]

Thanks ! I am going to test it. Do you mean that I should just do “git pull origin master” from my current branch ?

On 12 Oct 2016, at 4:30 PM, Ben Metcalf notifications@github.com wrote:

This might fix the problem with constructing from a parameter file.

Please merge only from the master. Do not merge between development branches. This will make it much more difficult to track down bugs.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/glenco/SLsimLib/pull/113#issuecomment-253151411, or mute the thread https://github.com/notifications/unsubscribe-auth/AG1NHjccvLHT8n2IVk6cstYt9DYhwg65ks5qzJqSgaJpZM4KP1C7.

[rbmetcalf]

No, you shouldn't merge with anything but the master. To test this PR you should checkout this branch and test it. When we think it works we will merge it into the master and you can merge the master into your branch.

[rbmetcalf]

Ideally you wouldn't even merge the master into your branch but rebase your branch onto the head of the master. Then the tree will be clean and it can be clearly seen in what order things cam into the code.

[rbmetcalf]

I made further changes. You can see that Dist is now assigned in Lens::insertMain() and Lens::replaceMain() so that this doesn't need to be done my hand.

[FabienNugier]

Hi. I went through your changes. Indeed that’s how I did it on my side for insertMainHalo and replaceMainHalos. I merged your commits into my branch and the code works fine. I guess we should very soon do a merge of my branch with the master branch. I think my branch is now more or less complete in the sense that it was made for the development of LoS and substructure (even if we focused on substructure), and it is doing what we want. Now everything should be working fine for me to run the code on different lenses :).

On 12 Oct 2016, at 8:18 PM, Ben Metcalf notifications@github.com wrote:

I made further changes. You can see that Dist is now assigned in Lens::insertMain() and Lens::replaceMain() so that this doesn't need to be done my hand.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/glenco/SLsimLib/pull/113#issuecomment-253197805, or mute the thread https://github.com/notifications/unsubscribe-auth/AG1NHursJIq0OrZPqNiOkc0pqide4-_Eks5qzNAUgaJpZM4KP1C7.

[FabienNugier]

Actually there seems to be a little problem with the lens defined from the parameter file. I have the impression that Lens does not know its redshift until we put a main halo in it. So the way to make the Lens properly defined is the following : define lens from parameter file (where parameter file contains the definition of a main halo) > Create main halo from parameter file > use setZlens to define zlens > call replaceMainHalos (to replace the main halo from the parameter file and set the lens redshift). If we use insertMainHalo instead of replaceMainHalos we get that zlens is not defined because I think the other halo from the parameter file is setting the redshift to zero (probably because the Lens did not read it). It is quite complicated.

On 13 Oct 2016, at 1:58 PM, Fabien Nugier fabien.nugier@googlemail.com wrote:

Hi. I went through your changes. Indeed that’s how I did it on my side for insertMainHalo and replaceMainHalos. I merged your commits into my branch and the code works fine. I guess we should very soon do a merge of my branch with the master branch. I think my branch is now more or less complete in the sense that it was made for the development of LoS and substructure (even if we focused on substructure), and it is doing what we want. Now everything should be working fine for me to run the code on different lenses :).

On 12 Oct 2016, at 8:18 PM, Ben Metcalf <notifications@github.com mailto:notifications@github.com> wrote:

I made further changes. You can see that Dist is now assigned in Lens::insertMain() and Lens::replaceMain() so that this doesn't need to be done my hand.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/glenco/SLsimLib/pull/113#issuecomment-253197805, or mute the thread https://github.com/notifications/unsubscribe-auth/AG1NHursJIq0OrZPqNiOkc0pqide4-_Eks5qzNAUgaJpZM4KP1C7.

[rbmetcalf]

The Lens shouldn't know the lens redshift until it has a halo in it. You mean that the main_zlens is not read from the parameter file? This must have changed? It had to of worked before. Do you know when it started?

[rbmetcalf]

After construction with Lens(InputParams& params, long* my_seed, const COSMOLOGY &cosmo, bool verbose = false) the lens does not contain a main_halo. Perhaps this should be changed along with adding the possibility of specifying no main halo in the parameter file.

[rbmetcalf]

I think I corrected the problem. The redshift of NSIE halos were set to very when they were read in from the parameter file.

[FabienNugier]

Hi Ben,

They were set to zero ?

I am not sure if that solved the problem because yesterday I pulled the master branch and merged it to mine and I was still having problems with the use of parameter file main_halo + lens. To make it work I had to use the following :

LensHaloRealNSIE ZeroMassHalo (paramsZeroMass); Lens Empty (paramsZeroMass, &seed); ZeroMassHalo.setZlens(zl); Empty.replaceMainHalos(&ZeroMassHalo);

If I don’t use this way the redshift of the lens does not seem to be set in the lens called “Empty” (even though it has been reading main_zlens in the parameter file, otherwise I would have got an error message).

I think we should make it work but for my code it is less of a concern now as dealing with many lenses like I do is not really compatible with the use of a parameter file (in which the main halo is defined once and for all, and also particularly the redshifts of the lens and the source).

Best regards, Fabien

On 13 Oct 2016, at 7:29 PM, Ben Metcalf notifications@github.com wrote:

I think I corrected the problem. The redshift of NSIE halos were set to very when they were read in from the parameter file.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/glenco/SLsimLib/pull/113#issuecomment-253487630, or mute the thread https://github.com/notifications/unsubscribe-auth/AG1NHn_Pes4FWodgs3tZBzWKooQH5OlTks5qzhYxgaJpZM4KP1C7.

[rbmetcalf]

You have been merging this branch with your branch and not testing it separately.
We don't know if it is a problem with this branch or with the combination of your branch and this one.

Why do you want the redshift to be set for a massless halo?

Send me the parameter file. You can past it in here I guess.

[FabienNugier]

Hi Ben,

I think it works with the parameter file, but we have to use the 4 lines I was copying from my code. What I should have said is that I find a bit strange that to set the redshift of the lens we need to call the function setZlens of a main halo and make this one replace the main halo from the parameter file. I thought the halo knows its redshift independently from its mass to be zero or not. Maybe there are some good reasons.

For my parameter file I use the following :

outputfile fullrange_test_256 # File that will contain the image magnifications

GENERAL

main_halo_on true

MAIN HALO : NSIE

main_DM_halo_type NSIE

main_mass 0. # virial mass of the halo in Msun/h

Be careful : …

main_sigma 300 # host lens velocity dispersion in km/s main_core 0.0e-5 # host core radius in Mpc
main_axis_ratio 0.7 # If we want an elliptical NSIE profile main_pos_angle 0. # It has to be in radian

1 radian = 0.01745327777

                # 90 deg = 1.570795
                # 45 deg = 0.7853975

main_slope 1. main_zlens 0.34 # redshift

main_Rmax 2.0 # in Mpc # Not necessary for NSIE !

main_concentration 4.43753

####### MultiLens model ##########

field_off true # turn off field halos. It will set flag_switch_field_off

REDSHIFTS ##### ## HAVE TO BE SET BY HAND FOR THE MOMENT !

zsource_reference 3.62 # source redshift z_source 3.62 # !! This is different WRT NFWpure.fits file !!

COSMOLOGY

We should not define it and use WMAP5yr in the constructor of the lens.

####### Substructure model ############

main_sub_Ndensity 0.0e6 # number densiy of substructure main_sub_beta -1.0
main_sub_alpha -1.9

main_sub_R_submax 0.5e-3

main_sub_mass_max 1.0e9 main_sub_mass_min 1.0e6 main_sub_type 1 # 1 is for power law, 0 for NFW

main_stars_N 1000000 # number of stars to be implanted main_stars_fraction 1.0 # stellar mass fraction main_stars_mass_function One # stellar mass function main_stars_mass 1.0 # star mass in solar masses

On 18 Oct 2016, at 3:30 PM, Ben Metcalf notifications@github.com wrote:

You have been merging this branch with your branch and not testing it separately.

We don't know if it is a problem with this branch or with the combination of your branch and this one.

Why do you want the redshift to be set for a massless halo?

Send me the parameter file. You can past it in here I guess.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/glenco/SLsimLib/pull/113#issuecomment-254428972, or mute the thread https://github.com/notifications/unsubscribe-auth/AG1NHh09Y_6ZYy-QJdvfGV5v_9-b5Y2dks5q1HWugaJpZM4KP1C7.

[rbmetcalf]

So this is an aesthetic point?

We could have Lens::replaceMainHalo() and Lens::insertMainHalo() take the redshift as a separate parameter and replace the redshift in the LensHalo. This way main() need not know that a LensHalo has it redshift in it.

[FabienNugier]

Hi Ben,

Yes that solution would probably make it more elegant and simple I think.

I can try to do it on a separate branch.

Best regards, Fabien

On 19 Oct 2016, at 10:34 PM, Ben Metcalf notifications@github.com wrote:

So this is an aesthetic point?

We could have Lens::replaceMainHalo() and Lens::insertMainHalo() take the redshift as a separate parameter and replace the redshift in the LensHalo. This way main() need not know that a LensHalo has it redshift in it.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/glenco/SLsimLib/pull/113#issuecomment-254831661, or mute the thread https://github.com/notifications/unsubscribe-auth/AG1NHpd-cbDIWGJfIoQ6SLFApc5VxRjyks5q1iqSgaJpZM4KP1C7.

[rbmetcalf]

This will effect other people. Lets do it on this branch and make sure it works properly before we merge this branch.

[rbmetcalf]

I see your changes on the new branch.

How about you use git cherry-pick to put that commit onto this branch. Your going to use this a lot to merge you branch eventually.