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

implements stara #195

Open
wants to merge 27 commits into
base: main
Choose a base branch
from
Open

implements stara #195

wants to merge 27 commits into from

Conversation

Deus1704
Copy link
Contributor

@Deus1704 Deus1704 commented Mar 31, 2024

PR Description

This PR Implements the STARA algorithm (#37) based upon @Cadair's implementation.
Example way of using stara [Old]:

import astropy.units as u
import sunpy.map
from sunkit_image.stara import *

#Assuming data is present in /data
maps = sunpy.map.Map("./data/hmi*")
maps = [m.resample((1024, 1024) * u.pix) for m in maps]

segs = get_segs(maps, limb_filter_value = 10* u.percent)

plot_sunspots(segs, maps)

Outputs:

image
image

Currently understanding figure tests, will add in due time. Till then suggestions are requested.

  • Write figure tests
  • Better function names/structure of stara API
  • Changelog
  • Add Example
  • Add the corresponding Magnetograms
  • Additional checks in the figure test.

@Deus1704
Copy link
Contributor Author

pre-commit.ci autofix

@Deus1704 Deus1704 marked this pull request as draft March 31, 2024 02:33
sunkit_image/stara.py Outdated Show resolved Hide resolved
sunkit_image/stara.py Outdated Show resolved Hide resolved
sunkit_image/stara.py Outdated Show resolved Hide resolved
sunkit_image/stara.py Outdated Show resolved Hide resolved
sunkit_image/stara.py Outdated Show resolved Hide resolved
@nabobalis
Copy link
Contributor

It would be better to run the pre commit locally before you commit the changes.

@Deus1704
Copy link
Contributor Author

Deus1704 commented Apr 1, 2024

It would be better to run the pre commit locally before you commit the changes.

The issue is that the pre-commit on running locally is changing files which I didn't even touch

sunkit_image/stara.py Outdated Show resolved Hide resolved
changelog/195.feature.rst Outdated Show resolved Hide resolved
sunkit_image/stara.py Outdated Show resolved Hide resolved
@nabobalis
Copy link
Contributor

It would be better to run the pre commit locally before you commit the changes.

The issue is that the pre-commit on running locally is changing files which I didn't even touch

What files?

@Deus1704
Copy link
Contributor Author

Deus1704 commented Apr 1, 2024

What files?

Files like, granule.py, radial.py and their corresponding test files.

@nabobalis
Copy link
Contributor

What files?

Files like, granule.py, radial.py and their corresponding test files.

What did it change?

@Deus1704
Copy link
Contributor Author

Deus1704 commented Apr 1, 2024

What did it change?

It changed the import order within all of these files. Specifically the position of import sunpy .

@Deus1704
Copy link
Contributor Author

Deus1704 commented May 31, 2024

This didn't mess up with the WCS when resampled:

hmi_map_original = sunpy.map.Map('hmi.ic_45s.2024.05.08_165530_TAI.continuum.fits')
hmi_downsampled = hmi_map_original.resample((128, 128) * u.pixel)
hmi_downsampled.peek()

image

@nabobalis
Copy link
Contributor

Can you see if that works with stara?

@Deus1704
Copy link
Contributor Author

Can you see if that works with stara?

This (128*128) one particularly has some issue with the stara. The error is RuntimeError: footprint array has incorrect shape. I'll try finding a workaround/reason for this error. Also this file(138KB) is slightly above the threshold size limit of 100KB

@nabobalis
Copy link
Contributor

Can you see if that works with stara?

This (128*128) one particularly has some issue with the stara. The error is RuntimeError: footprint array has incorrect shape. I'll try finding a workaround/reason for this error. Also this file(138KB) is slightly above the threshold size limit of 100KB

What is the full trace back?

@Deus1704
Copy link
Contributor Author

What is the full trace back?

RuntimeError                              Traceback (most recent call last)
/workspace/sunkit-image/rand.ipynb Cell 49 line 1
----> 1 segs= stara(hmi_hard, limb_filter=20*u.percent)

File /workspace/.pyenv_mirror/user/current/lib/python3.12/site-packages/astropy/units/decorators.py:313, in QuantityInput.__call__.<locals>.wrapper(*func_args, **func_kwargs)
    311 # Call the original function with any equivalencies in force.
    312 with add_enabled_equivalencies(self.equivalencies):
--> 313     return_ = wrapped_function(*func_args, **func_kwargs)
    315 # Return
    316 ra = wrapped_signature.return_annotation

File /workspace/sunkit-image/sunkit_image/stara.py:71, in stara(smap, circle_radius, median_box, threshold, limb_filter)
     69 # Median filter to remove detections based on hot pixels
     70 m_pix = int((median_box / smap.scale[0]).to_value(u.pix))
---> 71 med = median(data, square(m_pix), behavior="ndimage")
     73 # Construct the pixel structuring element
     74 c_pix = int((circle_radius / smap.scale[0]).to_value(u.pix))

File /workspace/.pyenv_mirror/user/current/lib/python3.12/site-packages/skimage/filters/_median.py:80, in median(image, footprint, out, mode, cval, behavior)
     78 if footprint is None:
     79     footprint = ndi.generate_binary_structure(image.ndim, image.ndim)
---> 80 return ndi.median_filter(
     81     image, footprint=footprint, output=out, mode=mode, cval=cval
     82 )

File /workspace/.pyenv_mirror/user/current/lib/python3.12/site-packages/scipy/ndimage/_filters.py:1588, in median_filter(input, size, footprint, output, mode, cval, origin, axes)
   1541 @_ni_docstrings.docfiller
   1542 def median_filter(input, size=None, footprint=None, output=None,
   1543                   mode="reflect", cval=0.0, origin=0, *, axes=None):
   1544     """
   1545     Calculate a multidimensional median filter.
   1546 
   (...)
   1586     >>> plt.show()
   1587     """
-> 1588     return _rank_filter(input, 0, size, footprint, output, mode, cval,
   1589                         origin, 'median', axes=axes)

File /workspace/.pyenv_mirror/user/current/lib/python3.12/site-packages/scipy/ndimage/_filters.py:1448, in _rank_filter(input, rank, size, footprint, output, mode, cval, origin, operation, axes)
   1446 fshape = [ii for ii in footprint.shape if ii > 0]
   1447 if len(fshape) != input.ndim:
-> 1448     raise RuntimeError('footprint array has incorrect shape.')
   1449 for origin, lenf in zip(origins, fshape):
   1450     if (lenf // 2 + origin < 0) or (lenf // 2 + origin >= lenf):

RuntimeError: footprint array has incorrect shape.

@nabobalis
Copy link
Contributor

Can you check if its 256 by 256 or 512 by 512?

Is the code only going to work if its 4k by 4k?

@Deus1704
Copy link
Contributor Author

Can you check if its 256 by 256 or 512 by 512?

Is the code only going to work if its 4k by 4k?

It works if its anything >= 256 by 256 pixels.

@nabobalis
Copy link
Contributor

Can you check if its 256 by 256 or 512 by 512?
Is the code only going to work if its 4k by 4k?

It works if its anything >= 256 by 256 pixels.

Should a comment to the function about this.

So how large is the HMI image if its 256 by 256?

Could we store the 128 by 128 file and then just upscale it?

@Deus1704
Copy link
Contributor Author

Deus1704 commented May 31, 2024

Should a comment to the function about this.

Comment like "the data array provided to the stara should be larger than 128*128 pixels" ?

So how large is the HMI image if its 256 by 256?

523KB

Could we store the 128 by 128 file and then just upscale it?

This is really great idea, it works! Only issue with 128 by 128 pixels now would be the size of it (138KB). But we could still create 64 by 64 (which would be around 42KB)and then upsample it. Tried this, it works too.

@nabobalis
Copy link
Contributor

Should a comment to the function about this.

Comment like "the data array provided to the stara should be larger than 128*128 pixels" ?

That works.

So how large is the HMI image if its 256 by 256?

523KB

That is too large.

Could we store the 128 by 128 file and then just upscale it?

This is really great idea, it works! Only issue with 128 by 128 pixels now would be the size of it (138KB). But we could still create 64 by 64 (which would be around 42KB)and then upsample it. Tried this, it works too.

How does the 64 by 64 one look and how does it look upscaled?

I am very curious.

@Deus1704
Copy link
Contributor Author

How does the 64 by 64 one look and how does it look upscaled?

I am very curious.

this is 64 by 64
image

while this is the upscaled 512 by 512
image

@nabobalis
Copy link
Contributor

Those look really rough but as long as they work!

@Deus1704
Copy link
Contributor Author

Deus1704 commented Jun 6, 2024

I have finally updated the figure test for this. Have a look at these images created from the data array in conftest
before
image
after
image

While I have just extracted the data from a small HMI map, the data array looks big due to pre-commit's linting style. I will although try to figure out some more elegant way but for the time being this would be fine?

@nabobalis
Copy link
Contributor

Could the data be a FITS file instead?

"car_rot": 2284,
}

return sunpy.map.GenericMap(data, header)
Copy link
Contributor

Choose a reason for hiding this comment

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

Can this file not be saved as a file via Maps save function?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

it can be saved

@Deus1704
Copy link
Contributor Author

Deus1704 commented Jun 6, 2024

Could the data be a FITS file instead?

yeah it can be but I thought we agreed to create a test case and not a fits file when discussed due to issues with size?

@nabobalis
Copy link
Contributor

Could the data be a FITS file instead?

yeah it can be but I thought we agreed to create a test case and not a fits file when discussed due to issues with size?

I want to avoid a large file for sure. How large would this be?

assert isinstance(result, np.ndarray)
assert result.shape == hmi_upscaled.data.shape
total_true_value_count = np.array(result)
assert total_true_value_count.sum() == 5516
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is weird since when I run this on my local machine this test passes.
This test is actually counting the total number of True values in the 2D array result .

@Deus1704
Copy link
Contributor Author

Deus1704 commented Jun 6, 2024

I want to avoid a large file for sure. How large would this be?

Its really small, 5.6 KB

@nabobalis
Copy link
Contributor

I want to avoid a large file for sure. How large would this be?

Its really small, 5.6 KB

So we got two choices:

Check in the fits file since it's tiny.

Or

Move the array data into a text file that is read by the fixture to create the map.

@Deus1704
Copy link
Contributor Author

Deus1704 commented Jun 6, 2024

So we got two choices:

Check in the fits file since it's tiny.

Or

Move the array data into a text file that is read by the fixture to create the map.

I would prefer the first way but would that(fits file) reside in sunpy's test folder or in sunkit-image?

@nabobalis
Copy link
Contributor

I would prefer the first way but would that(fits file) reside in sunpy's test folder or in sunkit-image?

Assuming we have the infrastructure setup, I would prefer it in this repo.

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

2 participants