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

Implement plane_to_sphere #186

Merged
merged 6 commits into from
Jun 6, 2023
Merged

Conversation

milancurcic
Copy link
Member

No description provided.

@milancurcic milancurcic added enhancement New feature or request archive-label-analysis-functions Oceanographic Lagrangian analysis functions labels Jun 2, 2023
@milancurcic milancurcic requested a review from philippemiron June 2, 2023 20:46
@milancurcic
Copy link
Member Author

Recall that we did that in #183 following your suggestion. While working on this PR, I realized that there is significant precision loss (O(1 m)) when going from sphere to plane for large distances from origin. This precision loss is independent from the precision of the input. In other words, even when the input is 32-bit, doing the calculation and returning the output in 32-bit results in a precision loss compared to working with 64-bit for calculation and output. So, relative to #183, I changed my mind and decided to work in 64-bit regardless of input type. But this is a question of design: We can choose to be (a) type-consistent with the input (i.e. use np.empty_like()) but document possible precision loss; or (b) always ensure higher precision (algorithm inaccuracies aside) and document that the input-output types may be inconsistent. I don't know what's better and I like them both for different reasons. What do you think?

@philippemiron
Copy link
Contributor

philippemiron commented Jun 6, 2023

By default in Python floating numbers are in float64, and I guess most people use it like this since you don't have to define the types. The reason my suggestion would be to keep type is that if it's float32, I would guess the user switched it to save space, to be compatible with another app (pytorch for example), etc.; modifying the type back to float64 might create more confusion having different inputs/outputs.

We can add a note that round-trip of positions on the Earth are not exact if precision is decreased. But at the same time, this is almost always the case for any function.

For example:

In [1]: np.arange(0, 10, 0.45)
Out[1]: 
array([0.  , 0.45, 0.9 , 1.35, 1.8 , 2.25, 2.7 , 3.15, 3.6 , 4.05, 4.5 ,
       4.95, 5.4 , 5.85, 6.3 , 6.75, 7.2 , 7.65, 8.1 , 8.55, 9.  , 9.45,
       9.9 ])

In [2]: np.arange(np.float32(0), np.float32(10), np.float32(0.45))
Out[2]: 
array([0.        , 0.44999999, 0.89999998, 1.34999996, 1.79999995,
       2.24999994, 2.69999993, 3.14999992, 3.5999999 , 4.04999989,
       4.49999988, 4.94999987, 5.39999986, 5.84999985, 6.29999983,
       6.74999982, 7.19999981, 7.6499998 , 8.09999979, 8.54999977,
       8.99999976, 9.44999975, 9.89999974])

milancurcic and others added 3 commits June 6, 2023 14:55
Co-authored-by: Philippe Miron <philippemiron@gmail.com>
Co-authored-by: Philippe Miron <philippemiron@gmail.com>
@milancurcic
Copy link
Member Author

Thanks for the review.

@milancurcic milancurcic merged commit 6acab3b into Cloud-Drift:main Jun 6, 2023
@milancurcic milancurcic deleted the plane-to-sphere branch June 6, 2023 19:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
archive-label-analysis-functions Oceanographic Lagrangian analysis functions enhancement New feature or request
Development

Successfully merging this pull request may close these issues.

2 participants