Skip to content

Add linear interpolation methods.#412

Merged
arjo129 merged 28 commits intomainfrom
arjo/feat/interpolation
May 2, 2022
Merged

Add linear interpolation methods.#412
arjo129 merged 28 commits intomainfrom
arjo/feat/interpolation

Conversation

@arjo129
Copy link
Copy Markdown
Contributor

@arjo129 arjo129 commented Apr 18, 2022 •
edited
Loading

🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸

🎉 New feature

This PR adds linear interpolation methods to the ignition math library these interpolation methods are needed for the VolumetricScalarGrid field,.

Signed-off-by: Arjo Chakravarty arjo@openrobotics.org

Checklist

  • Signed all commits for DCO
  • Added tests
  • Added example and/or tutorial
  • Updated documentation (as needed)
  • Updated migration guide (as needed)
  • codecheck passed (See contributing)
  • All tests passed (See test coverage)
  • While waiting for a review on your PR, please help review another open pull request to support the maintainers

Note to maintainers: Remember to use Squash-Merge and edit the commit message to match the pull request summary while retaining Signed-off-by messages.

arjo129 added 10 commits April 8, 2022 19:42
@arjo129
Adds a volumetric grid field to help with lookup in 3D.
6d11ecc 
The following PR adds a datastructure that represents a sparse volumetric grid. It allows for storing data in 3D.

# TODO
- [ ] Write unit tests
- [ ] docs

Signed-off-by: Arjo Chakravarty <arjo@openrobotics.org>
@arjo129
docs
e86b8bd 
Signed-off-by: Arjo Chakravarty <arjo@openrobotics.org>
@arjo129
Move test to TEST file
c923825 
Signed-off-by: Arjo Chakravarty <arjo@openrobotics.org>
@arjo129
Add test cases
6c456cf 
Signed-off-by: Arjo Chakravarty <arjo@openrobotics.org>
@arjo129
codecheck
e58d632 
Signed-off-by: Arjo Chakravarty <arjo@openrobotics.org>
@arjo129
fix rounding error
c66d414 
Signed-off-by: Arjo Chakravarty <arjo@openrobotics.org>
@arjo129
Increase coverage
cc43c1c 
Signed-off-by: Arjo Chakravarty <arjo@openrobotics.org>
@arjo129
Address Feedback
8707e2f 
Signed-off-by: Arjo Chakravarty <arjo@openrobotics.org>
@arjo129
Rework API to make it easier to use with interpolation.
ca1fc75 
Signed-off-by: Arjo Chakravarty <arjo@openrobotics.org>
@arjo129
Add linear interpolation methods.
feb5a43 
This PR adds linear interpolation methods to the ignition math library these interpolation methods are needed for the VolumetricScalarGrid field,.

Signed-off-by: Arjo Chakravarty <arjo@openrobotics.org>
@chapulina chapulina added the MBARI-LRAUV Sponsored by MBARI-LRAUV project: https://github.com/osrf/lrauv label Apr 18, 2022
arjo129 added 3 commits April 19, 2022 11:03
@arjo129
Update volumetric lookup field to use lookups
37a5280 
Signed-off-by: Arjo Chakravarty <arjo@openrobotics.org>
@arjo129
codecheck
aa1ccbb 
Signed-off-by: Arjo Chakravarty <arjo@openrobotics.org>
@arjo129
compile error.
8b53b4b 
Signed-off-by: Arjo Chakravarty <arjo@openrobotics.org>
@codecov
Copy link
Copy Markdown

codecov bot commented Apr 19, 2022 •
edited
Loading

Codecov Report

Merging #412 (4be2adb) into main (a8a5b94) will decrease coverage by 0.06%.
The diff coverage is n/a.

❗ Current head 4be2adb differs from pull request most recent head 116a045. Consider uploading reports for the commit 116a045 to get more accurate results

@@            Coverage Diff             @@
##             main     #412      +/-   ##
==========================================
- Coverage   99.90%   99.84%   -0.07%     
==========================================
  Files          46       49       +3     
  Lines        4260     4396     +136     
==========================================
+ Hits         4256     4389     +133     
- Misses          4        7       +3
Impacted Files Coverage Δ
...e/ignition/math7/ignition/math/detail/AxisIndex.hh 100.00% <0.00%> (ø)
...n/math7/ignition/math/VolumetricGridLookupField.hh 94.82% <0.00%> (ø)
...n/math7/ignition/math/detail/InterpolationPoint.hh 100.00% <0.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update a8a5b94...116a045. Read the comment docs.

This was referenced Apr 19, 2022
Closed
Merged
arjo129 added 3 commits April 21, 2022 11:19
@arjo129
Expose fewer features
ae0c62e 
Signed-off-by: Arjo Chakravarty <arjo@openrobotics.org>
@arjo129
docs
80e048b 
Signed-off-by: Arjo Chakravarty <arjo@openrobotics.org>
@arjo129
fix them compile issues
66c5ed0 
Signed-off-by: Arjo Chakravarty <arjo@openrobotics.org>
@arjo129 arjo129 changed the base branch from arjo/feat/volumetric_grid_field to main April 21, 2022 15:10
@arjo129 arjo129 marked this pull request as ready for review April 21, 2022 15:10
@arjo129 arjo129 requested a review from scpeters as a code owner April 21, 2022 15:10
@chapulina chapulina added the 🌱 garden Ignition Garden label Apr 22, 2022
hidmic
hidmic reviewed Apr 25, 2022
View reviewed changes
Copy link
Copy Markdown
Contributor

@hidmic hidmic left a comment

Choose a reason for hiding this comment

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

Another pass. Bottom line, code looks good but a few more tests would nice.

hidmic
hidmic reviewed Apr 25, 2022
View reviewed changes
arjo129 added 3 commits April 26, 2022 09:21
@arjo129
update them tolerances
076a40c 
Signed-off-by: Arjo Chakravarty <arjo@openrobotics.org>
@arjo129
Merge branch 'arjo/feat/interpolation' of https://github.com/ignition…
d4569a3 
…robotics/ign-math into arjo/feat/interpolation
@arjo129
Update tests
553ca3d 
Signed-off-by: Arjo Chakravarty <arjo@openrobotics.org>
arjo129 added 5 commits April 26, 2022 15:36
@arjo129
Add some assertions
a7be682 
Signed-off-by: Arjo Chakravarty <arjo@openrobotics.org>
@arjo129
For loop inside assertion
6afe983 
Signed-off-by: Arjo Chakravarty <arjo@openrobotics.org>
@arjo129
more warnings and assertions.
267cda3 
Signed-off-by: Arjo Chakravarty <arjo@openrobotics.org>
@arjo129
more assertions
dbc3261 
Signed-off-by: Arjo Chakravarty <arjo@openrobotics.org>
@arjo129
compiler error
895547d 
Signed-off-by: Arjo Chakravarty <arjo@openrobotics.org>
hidmic
hidmic approved these changes Apr 26, 2022
View reviewed changes
Copy link
Copy Markdown
Contributor

@hidmic hidmic left a comment

Choose a reason for hiding this comment

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

LGTM barring a few comments. Also, if we can further reduce all tolerances used in tests, that'd be great :)

include/ignition/math/detail/AxisIndex.hh
/// interpolation.
public: std::vector<InterpolationPoint1D<T>> GetInterpolators(
const T &_value,
double _tol = 1e-6) const
Copy link
Copy Markdown
Contributor

@hidmic hidmic Apr 26, 2022

Choose a reason for hiding this comment

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

@arjo129 should _tol be of type T?

include/ignition/math/detail/AxisIndex.hh
// Out of range
return {};
}
else if (fabs(it->first - _value) < _tol)
Copy link
Copy Markdown
Contributor

@hidmic hidmic Apr 26, 2022

Choose a reason for hiding this comment

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

@arjo129 nit^N!: if we use abs instead of fabs, including an using std::abs; statement before the if-else clause, T can be any numeric value, not just float and double.

@hidmic
Copy link
Copy Markdown
Contributor

hidmic commented Apr 26, 2022

CI failures seem unrelated. We are lacking some test coverage on VolumetricGridLookupField.hh though.

arjo129 added 2 commits April 27, 2022 15:23
@arjo129
Address some feedback
135ae32 
Signed-off-by: Arjo Chakravarty <arjo@openrobotics.org>
@arjo129
docs and coverage
bcc6b37 
Signed-off-by: Arjo Chakravarty <arjo@openrobotics.org>
@arjo129 arjo129 enabled auto-merge (squash) April 28, 2022 03:08
@arjo129 arjo129 merged commit 9622e9e into main May 2, 2022
@arjo129 arjo129 deleted the arjo/feat/interpolation branch May 2, 2022 19:01
@scpeters scpeters mentioned this pull request May 3, 2022
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@hidmic hidmic hidmic approved these changes

@scpeters scpeters Awaiting requested review from scpeters scpeters is a code owner

Assignees

No one assigned

Labels

🌱 garden Ignition Garden MBARI-LRAUV Sponsored by MBARI-LRAUV project: https://github.com/osrf/lrauv

Projects

Core development
Archived in project

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@arjo129 @hidmic @j-rivero @chapulina @osrf-triage @scpeters