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

Fix for irregular grid in FindLinearIdx #204

Merged
merged 6 commits into from
Apr 15, 2021

Conversation

WPringle
Copy link
Collaborator

Testing for irregular DEM grid in FindLinearIdx function in order tointerpolate properly using cell-averaged method for irregular DEM grid

…interpolate properly using cell-averaged method for irregular DEM grid
@krober10nd
Copy link
Collaborator

So it looks like there's three general cases:

  1. Uniform grid DEM with both axes featuring same grid spacings.
  2. Varying grid spacings in x and y axes.
  3. Varying grid spacings along the x/y axis itself potentially at the same time (this is the irregular case that did not work).

I'll test this this weekend and get back to you.

@WPringle
Copy link
Collaborator Author

WPringle commented Mar 26, 2021

Yes, that's right.

I made a test (just modification of Example2) using the DEM used in the issue #203 and it seemed to fix that alignment using the updated varying grid spacings along the x/y axes.

@krober10nd
Copy link
Collaborator

Could you send me this test please?

@krober10nd
Copy link
Collaborator

Thinking about this logic, seems maybe a bit faulty.

if max(dx) ~= min(dx)

This also could be the case for dy.

I would make this

if max(dx) ~= min(dx) || max(dy) ~= min(dy)

@WPringle
Copy link
Collaborator Author

True that's a good change

@krober10nd
Copy link
Collaborator

krober10nd commented Mar 26, 2021

what I would like to see is a test in the tests folder that demonstrates all three cases that I enumerated below work.
For example, generate a DEM from an analytical function and then build the mesh over and over again with finer solution and compare the interpolated bathymetry with the analytical function.

@krober10nd
Copy link
Collaborator

Given that this problem has now appeared three times, it's important this doesn't happen again.

@WPringle
Copy link
Collaborator Author

fair enough, I can work on that.

@krober10nd
Copy link
Collaborator

Let me know if you need any help

…me region but with different resolution types (dx=dy, dx~=dy and varying dx~=dy)
@WPringle
Copy link
Collaborator Author

Made a test using a subset of the CUDEM with different resolution types. Please check it out

@WPringle WPringle merged commit bda0bcb into Projection Apr 15, 2021
@WPringle WPringle deleted the interp_irregular_grid_fix branch April 15, 2021 22:34
@WPringle
Copy link
Collaborator Author

Made sure test does not pass with the previous interpolation method.

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.

2 participants