Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Robinson: remove unnecessary halfPi, use sign #211

Merged
merged 1 commit into from
Jun 8, 2021
Merged

Conversation

mrhso
Copy link
Contributor

@mrhso mrhso commented Jun 6, 2021

No description provided.

@Fil Fil self-requested a review June 6, 2021 16:47
Copy link
Member

@Fil Fil left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm surprised by the value
1.5933781076941205
since
1.593415793900743 = 1.0144 * Math.PI / 2

@mrhso
Copy link
Contributor Author

mrhso commented Jun 8, 2021

I'm surprised by the value
1.5933781076941205
since
1.593415793900743 = 1.0144 * Math.PI / 2

According to the original algorithm of the Robinson projection:
x = 0.8487RXλ
y = 1.3523RY

The scale factor should be 1.3523 / 0.8487.
(1.3523 / 0.8487) / (π / 2) ≐ 1.0144

@Fil
Copy link
Member

Fil commented Jun 8, 2021

The difference between the two constants is 0.002%, smaller than the precision of the numbers in the original description, but enough to make the regression test fail. So if we wanted to change that, we'd need to justify it in some way, probably because we'd want to follow the original definition, or a standard.

In other words, my question should have been: is this PR about the simplification of the code (in which case the constant should be 1.593415793900743, and the projection unchanged). Or is it also about enforcing a (very slightly) different aspect ratio (in which case the constant could be written explicitly as 1.3523 / 0.8487, and the reference image updated).

I don't have access to the original Robinson paper (1974) to double check, but Snyder 1990 (who gives a fortran program with 1.3523, 0.8487 and 0.5072 (ie 1.0144/2)), quotes from it:

"The length of the equator of the projection is 0.8487 times the circumference of a sphere of equal area, and the length of the central meridian of the projection is 0.5072 times the length of the equator of the projection" (Robinson
1974, 151)

In my understanding, this means that the current code already follows Robinson's original formulation (the aspect ratio is 1:0.5072), and our constant should be defined as 0.5072 * π (at our precision, 1.593415793900743). 1.3523 is just an approximation to 1e-5 derived by Snyder, and not part of the original formulation; it should be 0.8487 * 0.5072 * π = 1.35233…

@mrhso
Copy link
Contributor Author

mrhso commented Jun 8, 2021

Changed.

@Fil Fil merged commit 7894c74 into d3:master Jun 8, 2021
@Fil
Copy link
Member

Fil commented Jun 8, 2021

thanks! That was a nice opportunity to read more about this very classical projection :)

@mrhso mrhso deleted the robinson branch June 8, 2021 08:39
mrhso added a commit to mrhso/d3-geo-projection that referenced this pull request May 29, 2022
Fil pushed a commit that referenced this pull request May 29, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants