search

catherinezucker / radfil

Building Radial Density Profiles for Interstellar Filaments
GNU General Public License v3.0
14 stars 8 forks source link

Another bug in build_profile? #17

Closed catherinezucker closed 7 years ago

catherinezucker commented 7 years ago

Hi @hopehhchen ,

I think I might have found another bug in the newest version of build_profile.

img,hdr=fits.getdata("./Fil10_Bug_Data/HIGAL_L_Fil10_N.fits",header=True) mask=fits.getdata("./Fil10_Bug_Data/Fil10_filfind_mask.fits") dist=3200.

fil=radfil_class.radfil(img,filmask.astype(bool),header=hdr,distance=dist,padsize=padsize,filspine=fits.getdata(lpath+"Skeleton_Longpaths/"+filname+"_longpath.fits")) fil.build_profile(pts_mask=pts_mask,cutdist=5.0,samp_int=int(np.round(0.5/imgscale))) fil.fit_profile(fitfunc="Plummer",fitdist=4.0,bgdist=(3.0,4.0)) fig=plt.figure() plt.scatter(fil.xall,fil.yall)

And then I get a single cut that looks like this:

screen shot 2017-08-17 at 4 46 55 pm

Do you think this is user error or an issue with the masking? And do you think it will affect the fit results? I am trying to get another draft out to Alyssa before I leave for Chile on Saturday. After I finish implementing all the other changes on the draft I can come back to this, but if you have time to take a look tonight that would be great too :)

hopehhchen commented 7 years ago

Ha, this looks like a real one! Could you please pass me the data (the image, the mask, and the spine), @catherinezucker? I don't think the fit will be affected by a lot but it surely will, since the code does not remove outliers when fitting for the profile. I'll try to sort it out tonight. :)

catherinezucker commented 7 years ago

Actually, I think this might be user error after all. Let me check something again before you do anything :).

Best, Catherine

hopehhchen commented 7 years ago

It looks like those zero points could be due to padding to me. Anyway, let me know if you think it's just bad input parameters. Thanks!

catherinezucker commented 7 years ago

Yeah, I just tested this and it definitely has to do with pad. Do you think there's any reason to keep pad? It seems like it might just cause bad things to happen. It was necessary before because of how I was building profiles, but not I think it's defunct. Are there any cases you can think of where people would want to add zeros in the new version?

hopehhchen commented 7 years ago

No, I actually wanted to remove it when I started the new RadFil. If you agree, I'll go ahead and remove padding all together.

catherinezucker commented 7 years ago

Yeah, I agree. If it's easy for you, that would be great. Otherwise, I can do it. Thanks so much, Hope!!

On Thu, Aug 17, 2017 at 5:38 PM, Hope Chen notifications@github.com wrote:

No, I actually wanted to remove it when I started the new RadFil. If you agree, I'll go ahead and remove padding all together.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/catherinezucker/radfil/issues/17#issuecomment-323201599, or mute the thread https://github.com/notifications/unsubscribe-auth/ARfXSbX3mhW06BZwuXLhOpb1nvlnRACeks5sZLLagaJpZM4O6y-J .

hopehhchen commented 7 years ago

I can do it. Thanks for pointing this out!

hopehhchen commented 7 years ago

Padding is now completely removed. See 8788b3b600f896cd0217afa153355913555737f7.

catherinezucker commented 7 years ago

Perfect! Thanks, Hope!

On Fri, Aug 18, 2017 at 11:45 AM, Hope Chen notifications@github.com wrote:

Closed #17 https://github.com/catherinezucker/radfil/issues/17.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/catherinezucker/radfil/issues/17#event-1212121745, or mute the thread https://github.com/notifications/unsubscribe-auth/ARfXSUa1F7kQkAwW3DOLOIijGcIZyLH4ks5sZbGHgaJpZM4O6y-J .