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

The start of a normalization module #964

Closed
wants to merge 13 commits into from
Closed

The start of a normalization module #964

wants to merge 13 commits into from

Conversation

nabobalis
Copy link
Contributor

The implementation of this http://arxiv.org/pdf/1403.6613v1.pdf

Eventually, I will add more methods.

@Cadair
Copy link
Member

Cadair commented May 3, 2014

ping @rhewett

I think we could add other "normal" normalisation methods here, maybe sqrt and gamma as two very easy ones?

@Cadair Cadair added this to the 0.5 milestone May 3, 2014
@nabobalis
Copy link
Contributor Author

Also for comparison with the PDF, see

Sun

@larrymanley
Copy link
Contributor

Recently read that paper. What's the proper series of imports to try this out?

For most purposes, the weights can be set
equal for all scales.
"""
if not weights:
Copy link
Member

Choose a reason for hiding this comment

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

Newline before here

@nabobalis
Copy link
Contributor Author

@larrymanley

Scipy, numpy are the only external python dependencies.

@rhewett Added some new lines!

@Cadair
Copy link
Member

Cadair commented May 3, 2014

@larrymanley

from sunpy.image.normalization import multiscale_gaussian

You will have to pull @nabobalis's branch to test it before we merge it though.

@larrymanley
Copy link
Contributor

figure_1
Note: _get_mpl_normalizer in sdo.py is preventing peek and plot from displaying anything but black with this and my 'intscale'

@nabobalis
Copy link
Contributor Author

Yeah. I've not tested the output with the other normalise functions. There will be nans in the output from the function.

Could that be the issue?

@@ -22,7 +22,7 @@ def resample_method(method):
assert resample_meta((2056, 2056), method, True, True) == (2056, 2056)

def test_resample_neighbor():
resample_method('neighbor')
resample_method('neighbor')
Copy link
Member

Choose a reason for hiding this comment

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

There are tabs everywhere in this file, so you just made it inconsistent. I will fix the whole file in an extra PR, I think it's cleaner if you leave like it was before.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'll put it back.

@Cadair
Copy link
Member

Cadair commented May 3, 2014

@nabobalis the easiest thing to do would be suppress the nan's but I don't know if that is sensible.

Maybe the _mpl_norm... method needs to use np.nanmax and np.nanmin?

@larrymanley
Copy link
Contributor

@nabobalis NANs aren't the issue. I wrote an equivalent to 'aia_intscale.pro' (not contributed yet) to do the LMSAL "standard" intensity scaling and clipping for the visuals, had to remove _get_mpl_normalizer on my local code for peek and plot to work.

@Cadair
Copy link
Member

Cadair commented May 3, 2014

@larrymanley oh so it is intscale that is breaking _get_mpl_normalizer

p.s. use ` for the code type setting.

The value used to calulcate the global gamma-transformed image.
Ideally should be betwen 2.5 to 4.

h : float, optional
Copy link
Member

Choose a reason for hiding this comment

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

This is not descriptive enough.

@larrymanley
Copy link
Contributor

@Cadair With regard to peek and plot, _get_mpl_normalizer is breaking intscale and multiscale_gaussian


Returns
-------
image: numpy.ndarray
Copy link
Member

Choose a reason for hiding this comment

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

space before :

@Cadair
Copy link
Member

Cadair commented May 3, 2014

@larrymanley Interesting.

@nabobalis can you see if you can work out why this is breaking _mpl_norm..?

import scipy.ndimage as ndimage


def multiscale_gaussian(data, sigma=[1.25, 2.5, 5, 10, 20, 40], k=0.7,
Copy link
Member

Choose a reason for hiding this comment

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

OK, so I've sorted this some. It needs to be more clear that this sigma is the $w$ from the paper, this should be in the docs. It should also be noted that this is in pixel coordinates. Perhaps if the input is a map with plate scale info a flag can force it to use physical widths? (perhaps that flag should default to true if it can be done?)

I'd also note in the docs that this is for visualization only.

Copy link
Member

Choose a reason for hiding this comment

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

sigma takes a list and should be sigmas to be consistent with weights.

Also, the inputs are not sanitized. Does it break if the argument sigma=7 is passed? It should, because 7 is not iterable.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, the code does break if sigma=7 is passed.

It breaks at the first if statement.

    if not weights:
        weights = np.ones(len(sigmas))

@rhewett
Copy link
Member

rhewett commented May 4, 2014

I'm wondering if these visualization specific normalization functions should be under either sunpy.vis.image.normalization or sunpy.image.vis.normalization

@Cadair
Copy link
Member

Cadair commented May 4, 2014

@rhewett that might not be a bad idea, make it clearer it is a vis tool

@nabobalis
Copy link
Contributor Author

So, what would be better, sunpy.vis.image.normalization or sunpy.image.vis.normalization?

@nabobalis
Copy link
Contributor Author

@larrymanley

How are you crating the new map for the gaussian processed data?

@Cadair
Copy link
Member

Cadair commented May 12, 2014

@nabobalis I think sunpy.visualization.normalization is the best place for this.

@Cadair Cadair removed this from the 0.5 milestone May 27, 2014
@coveralls
Copy link

Coverage Status

Coverage increased (+0.06%) when pulling e45efb1 on nabobalis:master into 011b596 on sunpy:master.

@nabobalis
Copy link
Contributor Author

ITS DEAD JIM!

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.

None yet

6 participants