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

Polar Plotting #796

Merged
merged 5 commits into from
Nov 1, 2023
Merged

Polar Plotting #796

merged 5 commits into from
Nov 1, 2023

Conversation

gawebb-dstl
Copy link
Contributor

@gawebb-dstl gawebb-dstl commented Apr 26, 2023

Create polar plots using plotly like this

@mharris-dstl mharris-dstl self-assigned this Apr 26, 2023
@gawebb-dstl gawebb-dstl marked this pull request as ready for review August 14, 2023 13:51
@gawebb-dstl gawebb-dstl requested a review from a team as a code owner August 14, 2023 13:51
@gawebb-dstl gawebb-dstl requested review from hpritchett-dstl and mharris-dstl and removed request for a team August 14, 2023 13:51
Copy link
Contributor

@mharris-dstl mharris-dstl left a comment

Choose a reason for hiding this comment

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

I've pushed some grammar changes (thought it was easier for everyone if I add them myself and put them in as one) but have some other thoughts for the example.

  • It needs a cover image putting in.
  • It probably needs to show tracks being plotted, since currently the example shows only ground truth and detections but there is functionality in the source code for track plotting.
  • The polar plots themselves could do with some more description in the titles. While polar plots look really cool, I personally have to concentrate quite hard to understand the information they're showing. Better descriptions would help a lot with that.
  • The scenario has 2 targets in 3D moving in all 3 dimensions, but the static plot only shows 2 dimensions.
  • Could potentially get a 2D animated plotter in there as well? I tried to add this but it hasn't got functionality to automatically plot moving platforms. But an animation of some sort would give a lot of intuition as to how the targets are moving.

@gawebb-dstl
Copy link
Contributor Author

  • It needs a cover image putting in.

I think this will be done automatically by read the docs

  • It probably needs to show tracks being plotted, since currently the example shows only ground truth and detections but there is functionality in the source code for track plotting.

There is PR in the pipeline to do tracking in angular state space. That will give us angle tracks. In that PR I'll update this example.

  • The polar plots themselves could do with some more description in the titles. While polar plots look really cool, I personally have to concentrate quite hard to understand the information they're showing. Better descriptions would help a lot with that.

Agreed. I'll update the branch with that

  • The scenario has 2 targets in 3D moving in all 3 dimensions, but the static plot only shows 2 dimensions.

I'll update the example to put the scenario in 2D

  • Could potentially get a 2D animated plotter in there as well? I tried to add this but it hasn't got functionality to automatically plot moving platforms. But an animation of some sort would give a lot of intuition as to how the targets are moving.

I may look into this for a later PR

docs/examples/Polar_Plotting.py Outdated Show resolved Hide resolved
docs/examples/Polar_Plotting.py Show resolved Hide resolved
docs/examples/Polar_Plotting.py Outdated Show resolved Hide resolved
docs/examples/Polar_Plotting.py Outdated Show resolved Hide resolved
docs/examples/Polar_Plotting.py Outdated Show resolved Hide resolved
stonesoup/plotter.py Outdated Show resolved Hide resolved
stonesoup/plotter.py Outdated Show resolved Hide resolved
stonesoup/plotter.py Outdated Show resolved Hide resolved
stonesoup/plotter.py Outdated Show resolved Hide resolved
stonesoup/plotter.py Outdated Show resolved Hide resolved
gawebb-dstl and others added 2 commits October 5, 2023 12:48
Co-authored-by: mpharris <mpharris@dstl.gov.uk>
Added examples of time vs Statevector[mapping] in polar plot example
docs/examples/Polar_Plotting.py Outdated Show resolved Hide resolved
Co-authored-by: Michael Harris <117291860+mharris-dstl@users.noreply.github.com>
Co-authored-by: Henry Pritchett <87075245+hpritchett-dstl@users.noreply.github.com>
Comment on lines +197 to +199
def to_plotly_json(self):
return float(self)

Copy link
Member

Choose a reason for hiding this comment

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

Is this used?

Copy link
Contributor Author

@gawebb-dstl gawebb-dstl Oct 24, 2023

Choose a reason for hiding this comment

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

Yes, I believe it is needed to plot angles when using Plotly

Copy link
Member

Choose a reason for hiding this comment

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

Would it be possible to use float() or .dtype(np.float_) in plotting calls instead?

Copy link
Contributor Author

@gawebb-dstl gawebb-dstl Oct 25, 2023

Choose a reason for hiding this comment

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

You could but you would have to repeat that code in every plot_### function in the Plottery and PolarPlotterly classes. Plotting Angle types using Plotly fails on my machine without it. The to_plotly_json function isn't called directly anywhere in stone-soup but it is called by the plotly package.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The default function in plotly/_plotly_utils/utils.py tries to convert Angle types but fails currently. We could try and edit the Angle class so it is picked up by one of the other encode methods but this would probably raise other implications.

Link to default function code

Copy link
Member

Choose a reason for hiding this comment

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

Ah, I see. I'll merge now.

Thanks for link to the code. Looks like Plotly does have encode for Decimal type, but not for Real or other numbers, which would be useful to add. I may raise an issue with them.

@sdhiscocks sdhiscocks merged commit 783a2db into main Nov 1, 2023
6 checks passed
@sdhiscocks sdhiscocks deleted the polar_plotting branch November 1, 2023 11:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants