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

fixes for #165, #238, #210, #205 #241

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

sosey
Copy link
Member

@sosey sosey commented Oct 18, 2021

This PR attempts to fix issues with the 1D gaussian fits used in radial plots and line plots. As such it addresses some common issues in the following:

fixes #165
closes #238
closes #210
closes #205

I still need to clean up and there's likely some more minor changes to make before this PR is merged, but FWHM looks more appropriate.

@janerigby checkout this branch and let me know if you see improvement
image

@sosey
Copy link
Member Author

sosey commented Oct 21, 2021

@larrybradley will you look at the changes I made for the modeling and make sure I'm not doing something stupid?

@larrybradley
Copy link
Member

@sosey The Gaussian model (red line) in your figure above looks (by eye) to have a mean/center value of r > 0 (it appears to be sloping downward to the left at r=0). It should be fixed to have a mean/center at exactly r=0.

@janerigby
Copy link

Is this where I admit my ignorance, that I don't know how to check out a branch that addresses an issue?

@alanwatsonforster
Copy link

I checked out the branch, and it still doesn’t seem to work as it should. As commented above, the Gaussian is not centered on zero.
imexam copy

@prdurrell
Copy link

Hi - I wanted to check on updates to imexam to get correct radial profile FWHM's from 'r' command. How does one update from 0.9.1 to test?

@prdurrell
Copy link

prdurrell commented Apr 21, 2022

Hi -- I wanted to check back on this ticket -- I see it has been combined with other tickets, but I am still having trouble with imexam. I am now running imexam 0.9.1, and I am still getting erroneous results using 'r'. Using 'j' or 'k'(1D profiles along X, Y axes) seem to work fine. See the following -- this is for a star on a drizzled ACS/WFC image, so the FWHM should be a little over 2. I have no idea what 'r' is fitting to. I have included screenshots of the same object using 'r', 'j' and 'k' for reference.
Screen Shot 2022-04-21 at 9 51 13 AM

I am not sure if the centering is off, or what have you.  But the amplitude in rimexam is far above what is suggested in the other subroutines, as is the more obvious FWHM difference.    

Pat Durrell

Comment on lines +176 to +177
fixed = {'mean': True, 'amplitude': True}
bounds = {'amplitude': (flux.max() / 2., flux.max())}
Copy link
Contributor

Choose a reason for hiding this comment

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

I copied this setting to spacetelescope/jdaviz#1409

@larrybradley larrybradley removed their request for review May 8, 2023 16:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
6 participants