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

Adds persistence transform example to the gallery #137

Closed
wants to merge 16 commits into from

Conversation

Telomelonia
Copy link

@Telomelonia Telomelonia commented Mar 18, 2023

PR Description

These include persistence transform maps and mapcubes and overplotting of multiple maps in the same matplotlib Axes instance.

TODO

  • make dependency for mpl_animator
  • changelog
  • update confg.py for animation
  • use different way to plot rather than peek()

Fixes #76

@Telomelonia
Copy link
Author

pre-commit.ci autofix

@Telomelonia
Copy link
Author

pre-commit.ci autofix

@Telomelonia
Copy link
Author

Telomelonia commented Mar 18, 2023

@nabobalis @wafels
This PR is ready for merge, and all ci has passed.

changelog/137.doc.rst Outdated Show resolved Hide resolved
# To create a MapSequence, we can call Map directly but add in a
# keyword to output a MapSequence instead.

aiamapseq = sunpy.map.Map(AIA_193_CUTOUT01_IMAGE, AIA_193_CUTOUT02_IMAGE, AIA_193_CUTOUT03_IMAGE, cube=True)
Copy link
Contributor

Choose a reason for hiding this comment

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

Does this cube keyword work?

Copy link
Author

Choose a reason for hiding this comment

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

Hmm ... my bad, there is no impact from the cube keyword.

Copy link
Contributor

Choose a reason for hiding this comment

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

How do we make it a sequence?

aiamapseq = sunpy.map.Map(AIA_193_CUTOUT01_IMAGE, AIA_193_CUTOUT02_IMAGE, AIA_193_CUTOUT03_IMAGE, cube=True)

###############################################################################
# For a data set consisting of N images with intensity values I(x,y,t),
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe this should be rendered as math text

Copy link
Author

Choose a reason for hiding this comment

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

I think we need to change the docs/config.py so the gallery will have animations.

Hmm ... i will go through that 👍

# This plots the final mapsequence after implementing the persistence transform

result_mapseq = sunpy.map.MapSequence(persistence_maps)
result_mapseq.peek()
Copy link
Contributor

Choose a reason for hiding this comment

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

We should not be using peek to display any images.

@nabobalis
Copy link
Contributor

I think we need to change the docs/config.py so the gallery will have animations.

Telomelonia and others added 8 commits March 18, 2023 23:52
Co-authored-by: Nabil Freij <nabil.freij@gmail.com>
Co-authored-by: Nabil Freij <nabil.freij@gmail.com>
Co-authored-by: Nabil Freij <nabil.freij@gmail.com>
Co-authored-by: Nabil Freij <nabil.freij@gmail.com>
Co-authored-by: Nabil Freij <nabil.freij@gmail.com>
Co-authored-by: Nabil Freij <nabil.freij@gmail.com>
Co-authored-by: Nabil Freij <nabil.freij@gmail.com>
Co-authored-by: Nabil Freij <nabil.freij@gmail.com>
@Telomelonia
Copy link
Author

pre-commit.ci autofix

pre-commit-ci bot and others added 2 commits March 18, 2023 18:52
Co-authored-by: Nabil Freij <nabil.freij@gmail.com>
Comment on lines +34 to +35
for i, map_i in enumerate(aiamapseq[1:]):
per = np.array([aiamapseq[n].data for n in [i, i + 1]]).max(axis=0)
Copy link
Member

Choose a reason for hiding this comment

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

Surely we don't need two loops to do this...Can this operation not be vectorized?

Comment on lines +37 to +38
smap.plot_settings["cmap"] = cm.get_cmap("Greys_r")
smap.plot_settings["norm"] = colors.LogNorm(100, smap.max())
Copy link
Member

Choose a reason for hiding this comment

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

These two lines should be removed and the keyword arguments can just be passed directly when calling plot, e.g. aiamapseq.plot(cmap=cm.get_cmap('Greys_r'), norm=colors.LogNorm(100, smap.max()).

# For a data set consisting of N images with intensity values I(x,y,t),
# the Persistence Map Pn is a function of several arguments,
# namely intensity, location and time:
# Pn(x,y,tn)=Q(I(x,y,t≤tn))
Copy link
Member

Choose a reason for hiding this comment

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

What is Q here? the actual operation that is being performed needs a lot more explanation. @wafels could you provide a few references where this kind of operation is used in the literature?

Copy link
Member

Choose a reason for hiding this comment

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

Typically Q is the maximum function - i.e., find the maximum value for all values from 0 to t_{n}. For a reference - DOI 10.3847/0004-637X/825/1/27 .

Copy link
Author

Choose a reason for hiding this comment

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

Thanks for the reference

@Telomelonia
Copy link
Author

@wtbarnes @nabobalis
My bad while making a proposal; I found this was a part of the GSoC project. I am closing this right now.

@Telomelonia Telomelonia deleted the persistence_transform branch March 20, 2023 15:39
@nabobalis
Copy link
Contributor

nabobalis commented Mar 20, 2023

@wtbarnes @nabobalis My bad while making a proposal; I found this was a part of the GSoC project. I am closing this right now.

I added a range of sunkit-image issues in the proposal, in this case, I have no problem with continuing this pull request and we replace the issue in the proposal.

@Telomelonia Telomelonia restored the persistence_transform branch March 20, 2023 16:20
@Telomelonia
Copy link
Author

@nabobalis
Ohh nice, thanks for the green flag ✌️

@Telomelonia Telomelonia reopened this Mar 20, 2023
@Telomelonia Telomelonia marked this pull request as draft March 30, 2023 11:04
@nabobalis
Copy link
Contributor

Thanks for working on this @Telomelonia but since there has been no activity, I will be closing this.

If you want to continue working on this, please reopen this.

@nabobalis nabobalis closed this Aug 9, 2023
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.

Add persistence transform example to the gallery
4 participants