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

Rebase suggestions for updates to trilinear interpolation #156

Conversation

tfoote
Copy link
Collaborator

@tfoote tfoote commented Jan 28, 2022

This is a replacement for #142 to build on top of #127 instead of on top of #83

// TODO(tfoote) magic numbers
// Should be passed in and paramaterized based on the expected data
// distribution height or calculated from the grandularity of the dataset.
float slice_depth = 5; // 5 meter.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Hmm, I see how this would simplify the search, but I think it makes the algorithm less robust. As you said, 5 meters is a magic number. It is very specific to the one specific data set we have. Right now, we don't have a way of calculating the resolution, and we know that resolution will depend on where you are in the data. If we upstream with this specific resolution, it's not going to work in the general case. The current code works in a more general case, whatever the granularity the data. For that, I think the complexity is worth it. We're not running into performance issues to need to simplify it.

Copy link
Collaborator

@mabelzhang mabelzhang left a comment

Choose a reason for hiding this comment

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

I tried testing this on the Test case 1 in #127, the interpolation_prism.csv, with the following changes in buoyant_tethys.sdf, and I wasn't able to run it. When I unpause the simulation, I got a seg fault. Are you able to reproduce it? The seg fault doesn't happen in #127.

Uncomment this lat long, comment out all others:

      <!-- For simple_test.csv and interpolation_test.csv -->
      <latitude_deg>0</latitude_deg>
      <longitude_deg>0</longitude_deg>

Uncomment this csv file, comment out all others:

    <plugin
      filename="ScienceSensorsSystem"
      name="tethys::ScienceSensorsSystem">
      <data_path>interpolation_prism.csv</data_path>
    </plugin>

Modify the robot position:

    <include>
      <pose>4 -5 0 0 0 0</pose>
      <uri>tethys_equipped</uri>
    </include>

@arjo129
Copy link
Member

arjo129 commented May 9, 2022

Since we have switched to a new and faster science data lookup architecture, I am going to close this PR.

@arjo129 arjo129 closed this May 9, 2022
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.

3 participants