search

ejeschke / ginga

The Ginga astronomical FITS file viewer
BSD 3-Clause "New" or "Revised" License
120 stars 77 forks source link

Fix zscale autocuts algorithm #1042

Closed ejeschke closed 1 year ago

ejeschke commented 1 year ago
ejeschke commented 1 year ago

@pllim, see if this has the desired fix for your data. Especially if you have a real data file, do you get acceptable estimated cuts?

pllim commented 1 year ago

Thanks for the quick fix! I am swamped next week but I'll review the week after. At a glance, I like the code simplification. 😸

ejeschke commented 1 year ago

Slice 1 data range is really [0, 89] (inclusive).

Hmm, when I look at slice 1 of image_cube_hdu_obj_microns.fits (example problem image above) it looks like the min and max are approx 0.00435, 0.99. Am I looking at the same image you are?

pllim commented 1 year ago

Oh ooops... I think you are right. I must have misread. I used random generator that is from 0 to 1, I think.

pllim commented 1 year ago

Oh wait, no, my local file must had be generated inconsistently, let me regenerate it.

pllim commented 1 year ago

Okay, it appears much better when the values are between 0 and 1. Cut low is 0.5007 and cut high is 0.9273.

The image I used in https://github.com/ejeschke/ginga/pull/1042#pullrequestreview-1306386829 must had been generated by a simple np.arange to see how the elements are arranged.

ejeschke commented 1 year ago

The image I used in https://github.com/ejeschke/ginga/pull/1042#pullrequestreview-1306386829 must had been generated by a simple np.arange to see how the elements are arranged.

@pllim can you upload a problem image to the STScI box storage shared with me? I'm inclined to merge this, but I want to be able to reliably reproduce error we are fixing and make sure that the new method is working decently to do that.

pllim commented 1 year ago

@ejeschke , are you able to grab https://stsci.box.com/s/xnepo5x4tbidhwub5w4t28d3o09ackfh ?

ejeschke commented 1 year ago

@ejeschke , are you able to grab https://stsci.box.com/s/xnepo5x4tbidhwub5w4t28d3o09ackfh ?

Yes, thank you!

pllim commented 1 year ago

Cut low is 0.5007 and cut high is 0.9273.

Without this patch, but on the latest master branch, cut low is 0.03439 and cut high is 0.9939. The exception reported in #1041 is no more. Thanks!

ejeschke commented 1 year ago

on the latest master branch, cut low is 0.03439 and cut high is 0.9939

@pllim, with this now-modified PR, what are the cuts for the same image?

pllim commented 1 year ago

With this PR rebased on top of latest master, now I get cut low 0.03439 and cut high 0.9939, so basically this PR does not change anything except you now don't have to maintain your own zscale algorithm. 😸

ejeschke commented 1 year ago

Agree, but I want to check some of our images to make sure I don't see significant differences in cut values. If there are I may need to tweak this PR some more.

ejeschke commented 1 year ago

Seems to be some minor differences, but not enough to matter much. Merging.