Computing distances from z1 to z2

[astrozot]

Hi,

I am asking to include in the computation of various distances, the distances between two redshifts. This kind of computation is important for gravitational lensing and is indeed included already in the Python astrolib package.

Regards,
Marco Lombardi

[giordano]

Thank you so much for the contribution, much appreciated! I suggest to simplify the code a little bit to avoid a lot of duplication.

It would be also great if you could update the documentation in the README.md

[astrozot]

Dear Giordano,

thank you for your quick reply! Yes, indeed we could unify the code and use a single function with a default argument. The only issue is that the default argument has to be z₂, because the "natural" way of indicating the distances would be from z₁ to z₂, but, logically, it would be z₁. We should therefore do, within the various functions involved, something like

if z₂ == 0
    z₁, z₂ = 0, z₁

I am not entirely sure that is better than having the dispatch mechanism handling the one-redshift case: to me this looks more involved than defining a new (one line effectively) function. What do you think?

[giordano]

Maybe I'm missing something, but if we default z₂ to 0 then scale_factor(z₂) = 1 when not specified and it's the same thing as the existing method, why would we need to swap?

[astrozot]

Maybe I was unclear in my explanation. The current implementation, i.e. the signature

angular_diameter_dist(cosmo, z₁, z₂)

computes the angular diameter distance from redshift z₁ to redshift z₂. In this notation, the function already implemented in the library, that is the angular diameter distance to redshift z, is obtained by setting z₁ = 0. Thus, if the user needs to compute, say, the angular diameter distance for an object at redshift 1.3, (s)he has to enter

angular_diameter_dist(cosmo, 0, 1.3)

If we want the user to be able to use this function without the 0 argument (which surely is something desirable, also for compatibility with respect to the current implementation), we cannot just set a default value, because that would have to be z₂, while really we need to set the default value for z₁.

I see three possible solutions

1. Swap the arguments in the definition, and call angular_diameter_dist(cosmo, z₁, z₂=0) the angular diameter distance from redshift z₂ to redshift z₁. This works, but I see it as quite unnatural. Note also that this argument order is thee opposite of what done in all the implementations I know (see, e.g., the Python astropy package).

2. Keep the standard definition, so that angular_diameter_dist(cosmo, z₁, z₂=0) is the angular diameter distance from redshift z₁ to redshift z₂. Internally, swap the arguments z₁ and z₂ if z₂ == 0. This is OK, but is not too nice in my opinion. Also, it makes things a little complicated if z₁ and/or z₂ are vectors (incidentally, this is something that probably should be implemented somehow).

3. Use the multiple dispatch offered by Julia, and include two definitions, as I did in my proposal. I think this is the cleanest way, but I understand that might be just my personal point of view.

Regards,
Marco

[giordano]

I think the order of the arguments you suggest is totally reasonable, but the code duplication is a bit unfortunate.

Could you please add tests for the new method and address the few unresolved comments above? Then we can merge this and maybe I'll try to work out a way to reduce code duplication later.

Edit: ok, I have a working way to reduce the code duplication. I'm happy to merge this PR with the current implementation, provided that you add the tests.

[giordano]

Should luminosity_dist and distmod have the new method?

[astrozot]

Probably not: I never saw a luminosity distance between two different redshifts, and I cannot easily imagine a situation where such a quantity would be used. But, for completeness, one could also include it. FYI, the Python astrolib package does not include it.

Anyway, let me know your decision, and I will modify the code (and the Readme file) accordingly.

P.S. Not sure how it works: if I make a new commit to the code and push it to the repository, is the pull request automatically updated or should I make a new pull request?

[giordano]

Ok, no need to add these methods, but please add tests. You can just keep pushing to the same branch, the pull request will be updated

[astrozot]

Just to let you know that I pushed a commit with the fixes you requested (no deprecations for the new functions, readme updated, and new tests for the 2-redshift argument).

[giordano]

Awesome, thank you so much!

[astrozot]

Thank you for your quick reaction! I am happy I could contribute somehow (even if to such a small degree) to the library. Regards,

Marco