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

Constant latitude intersections using bounding box #1017

Merged
merged 14 commits into from
Nov 8, 2024

Conversation

philipc2
Copy link
Member

@philipc2 philipc2 commented Oct 14, 2024

Closes #1018

Overview

  • Uses the internally computed bounding box to compute the constant latitude intersections for each face

Usage

uxgrid = ...

# bounding box
uxgrid.get_faces_at_constant_latitude(lat=20.0, use_spherical_bounding_box=True)

# intersection of each edge
uxgrid.get_faces_at_constant_latitude(lat=20.0, use_spherical_bounding_box=True)

@philipc2 philipc2 self-assigned this Oct 14, 2024
@philipc2 philipc2 marked this pull request as ready for review October 14, 2024 21:50
@philipc2
Copy link
Member Author

@hongyuchen1030 @amberchen122

This implements _get_candidate_faces_at_constant_latitude from #785 using the desired API.

Let me know if method='bounding_box_intersection' is appropriate for this.

@philipc2
Copy link
Member Author

Feel free to merge any tests from #785 that are relevant to the candidate face computation also.

@hongyuchen1030
Copy link
Contributor

@hongyuchen1030 @amberchen122

This implements _get_candidate_faces_at_constant_latitude from #785 using the desired API.

Let me know if method='bounding_box_intersection' is appropriate for this.

Hi @philipc2
Thanks for putting this up together. I am extremely busy this week for my paper and my QE. I will try my best to review all the recent PRs this weekend.

However, I don't think method='bounding_box_intersection' is the proper name.

In fact, this is the only method we mathematically calculate the intersections correctly.

All other "faster" methods didn't guarantee the intersection points and just gave some approximate results that "might work".

So I feel like we should name this one similar to "mathematical intersections" and other methods as similar to "tentative methods".

@philipc2
Copy link
Member Author

@hongyuchen1030 @amberchen122

This implements _get_candidate_faces_at_constant_latitude from #785 using the desired API.

Let me know if method='bounding_box_intersection' is appropriate for this.

Hi @philipc2
Thanks for putting this up together. I am extremely busy this week for my paper and my QE. I will try my best to review all the recent PRs this weekend.

However, I don't think method='bounding_box_intersection' is the proper name.

In fact, this is the only method we mathematically calculate the intersections correctly.

All other "faster" methods didn't guarantee the intersection points and just gave some approximate results that "might work".

So I feel like we should name this one similar to "mathematical intersections" and other methods as similar to "tentative methods".

Thanks for the quick feedback! Good luck with your work this week!

@rajeeja
Copy link
Contributor

rajeeja commented Oct 15, 2024

Hey @philipc2 thanks for putting this together, would be the first step to determine the faces to perform area correction when edge is around line of constant latitude or great circle arc?

@philipc2
Copy link
Member Author

Hey @philipc2 thanks for putting this together, would be the first step to determine the faces to perform area correction when edge is around line of constant latitude or great circle arc?

I don't believe this would require any area correction, as the bounding box implemented by @hongyuchen1030 already considers the spherical geometry.

Though area calculation is definitely something that down the line is important, as a lot of calculations are area-weighted.

@hongyuchen1030
Copy link
Contributor

Hey @philipc2 thanks for putting this together, would be the first step to determine the faces to perform area correction when edge is around line of constant latitude or great circle arc?

I don't believe this would require any area correction, as the bounding box implemented by @hongyuchen1030 already considers the spherical geometry.

Though area calculation is definitely something that down the line is important, as a lot of calculations are area-weighted.

For non-conservative zonal average, we don't need to consider area calculation.

Area calculation is one of the last two steps of conservative zonal average.

uxarray/cross_sections/dataarray_accessor.py Outdated Show resolved Hide resolved
test/test_cross_sections.py Outdated Show resolved Hide resolved
uxarray/grid/intersections.py Outdated Show resolved Hide resolved
uxarray/grid/grid.py Outdated Show resolved Hide resolved
@philipc2 philipc2 changed the title Constant latitude intersections using bounding box Constant lat & lon intersections using bounding box Nov 7, 2024
@philipc2
Copy link
Member Author

philipc2 commented Nov 7, 2024

@hongyuchen1030

I'm looking to also implement constant longitude cross sections using the spherical bounding box.

The longitude bounds appear to be in the [0, 360] range. Do you have any suggestions on what I should do if I'd like to support the user to input a longitude value in the range [-180, 180].

I considered shifting it to [0, 360], but that didn't seem to fix the failing tests.

Longitudes for Face 0 (radians)
[-0.001 -0.003 -0.003 -0.001  0.002  0.002]
Bounds for Face 0 (radians)
[[-0.005  0.001]
 [ 6.28   0.002]]

Longitudes for Face 0 (degrees)
[-0.038 -0.186 -0.182 -0.048  0.1    0.096]
Bounds for Face 0 (degrees)
[[ -0.269   0.046]
 [359.814   0.1  ]]

Longitudes for Face 1 (radians)
[ 0.004  0.004  0.002 -0.001 -0.001  0.002]
Bounds for Face 1 (radians)
[[-0.001  0.005]
 [ 6.282  0.004]]

Longitudes for Face 1 (degrees)
[ 0.244  0.24   0.105 -0.043 -0.038  0.096]
Bounds for Face 1 (degrees)
[[ -0.037   0.278]
 [359.957   0.244]]

Longitudes for Face 2 (radians)
[-0.001 -0.001 -0.003 -0.006 -0.006 -0.003]
Bounds for Face 2 (radians)
[[-0.001  0.005]
 [ 6.278  6.283]]

Longitudes for Face 2 (degrees)
[-0.038 -0.043 -0.177 -0.325 -0.321 -0.186]
Bounds for Face 2 (degrees)
[[ -0.046   0.27 ]
 [359.675 359.962]]

Longitudes for Face 3 (radians)
[0.002 0.004 0.007 0.007 0.004 0.002]
Bounds for Face 3 (radians)
[[-0.005  0.001]
 [ 0.002  0.007]]

Longitudes for Face 3 (degrees)
[0.1   0.235 0.383 0.379 0.244 0.096]
Bounds for Face 3 (degrees)
[[-0.261  0.055]
 [ 0.096  0.383]]

@philipc2 philipc2 changed the title Constant lat & lon intersections using bounding box Constant latitude intersections using bounding box Nov 7, 2024
@philipc2
Copy link
Member Author

philipc2 commented Nov 7, 2024

@hongyuchen1030

Let's not tackle the constant longitude intersections here for the sake of getting this merged ASAP. We can open up an issue regarding the issues I'm having above.

For #785, we only need constant latitude intersections anyways.

@hongyuchen1030
Copy link
Contributor

hongyuchen1030 commented Nov 7, 2024

@hongyuchen1030

I'm looking to also implement constant longitude cross sections using the spherical bounding box.

The longitude bounds appear to be in the [0, 360] range. Do you have any suggestions on what I should do if I'd like to support the user to input a longitude value in the range [-180, 180].

I considered shifting it to [0, 360], but that didn't seem to fix the failing tests.

Longitudes for Face 0 (radians)
[-0.001 -0.003 -0.003 -0.001  0.002  0.002]
Bounds for Face 0 (radians)
[[-0.005  0.001]
 [ 6.28   0.002]]

Longitudes for Face 0 (degrees)
[-0.038 -0.186 -0.182 -0.048  0.1    0.096]
Bounds for Face 0 (degrees)
[[ -0.269   0.046]
 [359.814   0.1  ]]

Longitudes for Face 1 (radians)
[ 0.004  0.004  0.002 -0.001 -0.001  0.002]
Bounds for Face 1 (radians)
[[-0.001  0.005]
 [ 6.282  0.004]]

Longitudes for Face 1 (degrees)
[ 0.244  0.24   0.105 -0.043 -0.038  0.096]
Bounds for Face 1 (degrees)
[[ -0.037   0.278]
 [359.957   0.244]]

Longitudes for Face 2 (radians)
[-0.001 -0.001 -0.003 -0.006 -0.006 -0.003]
Bounds for Face 2 (radians)
[[-0.001  0.005]
 [ 6.278  6.283]]

Longitudes for Face 2 (degrees)
[-0.038 -0.043 -0.177 -0.325 -0.321 -0.186]
Bounds for Face 2 (degrees)
[[ -0.046   0.27 ]
 [359.675 359.962]]

Longitudes for Face 3 (radians)
[0.002 0.004 0.007 0.007 0.004 0.002]
Bounds for Face 3 (radians)
[[-0.005  0.001]
 [ 0.002  0.007]]

Longitudes for Face 3 (degrees)
[0.1   0.235 0.383 0.379 0.244 0.096]
Bounds for Face 3 (degrees)
[[-0.261  0.055]
 [ 0.096  0.383]]

Do you have any suggestions on what I should do if I'd like to support the user to input a longitude value in the range [-180, 180].

Just shift them to [0,360) internally.

I considered shifting it to [0, 360], but that didn't seem to fix the failing tests.

The range should be [0,360), so only one representation for the 0 longnitude. Also you can check if you represent the pole point uniformly

@philipc2 philipc2 merged commit 01efecd into main Nov 8, 2024
19 checks passed
@philipc2 philipc2 mentioned this pull request Nov 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Constant latitude cross sections using bounding box
3 participants