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

Interpolated Mass attenuation values don't seem to match the reference table data #24

Closed
samaloney opened this issue Aug 27, 2020 · 12 comments

Comments

@samaloney
Copy link
Contributor

Comparing the graphs from the NIST source page here the edges don't seem to be being interpolated properly.

Te_mass_atten

Code to create plot

e = np.linspace(1e-3, 20, 10000)*u.MeV
te = MassAttenuationCoefficient('Te')
plt.plot(e, te.func(e))
plt.loglog()
plt.xlim(1e-3, 1e2)
plt.ylim(1e-2, 1e4)
plt.xlabel('Photon Energy (MeV)')
plt.ylabel(r'$\mu / \rho$ (cm$^{2}$ g$^{-1}$)')
plt.title('Te')
@samaloney
Copy link
Contributor Author

It seems scipy's interp1d doesn't handle multivalued/non-monotonic x-values, attached plot shows the effect, evaluating the attenuation coefficient at same values used for the interpolation.

interp_issue

@rschwartz70
Copy link

where is the interp1d done in your codelet. If I could see the issue in detail I could determine whether my IDL code would suffer the same issue.

@samaloney
Copy link
Contributor Author

samaloney commented Aug 28, 2020

It's these lines, where self.energy.value and self.data.value are values from the NIST tables and the interpolation function is setup as self._f and then calling self._f(<log of engery>) returns the an interpolated value.

data_energy_kev = np.log10(self.energy.value)
data_attenuation_coeff = np.log10(self.data.value)
self._f = interpolate.interp1d(
data_energy_kev, data_attenuation_coeff, bounds_error=False,
fill_value=0.0
)
self.func = lambda x: u.Quantity(
10 ** self._f(np.log10(x.to("keV").value)), "cm^2/g"
)

I altered the first of any repeating data points by a tiny fraction (1e-10) and this resolved the issue but not sure if that is a good solution or not.

interp_fixed

@rschwartz70
Copy link

interp1d should be called using assume_sorted as true because the data are already sorted

@rschwartz70
Copy link

More extensive NIST tables exist into the pair production energy range. There are several independent processes that go into the mass atten coefficients that should be independently specifiable. It would require a trivial elaboration of this code. I am going to enlist Laura and Danny on this effort.

@ehsteve
Copy link
Owner

ehsteve commented Aug 31, 2020

Hey @rschwartz70 and @samaloney! sorry it took me so long to reply, I was on vacation last week. Thanks for noticing and reporting this bug. I had thought I had looked at this but apparently not closely enough (see https://roentgen.readthedocs.io/en/latest/examples/tutorial1.html)! It looks like @rschwartz70 solution works for me. I'll admit that I don't understand why it works because the documentation (https://docs.scipy.org/doc/scipy/reference/generated/scipy.interpolate.interp1d.html) states that it will sort the data itself by default so this should not be needed.

Figure_1

@ehsteve
Copy link
Owner

ehsteve commented Aug 31, 2020

@rschwartz70 I'd be happy to expand the data set here if you can point me to the right place. Can I ask what you guys are working on? I'm happy to see that some is beginning to use this!

@ehsteve ehsteve mentioned this issue Aug 31, 2020
@rschwartz70
Copy link

rschwartz70 commented Aug 31, 2020 via email

@ehsteve
Copy link
Owner

ehsteve commented Sep 1, 2020

@samaloney would you be willing to review my proposed fix in PR #25?

@samaloney
Copy link
Contributor Author

Just a final note on this the assume_sorted keyword only works for floats which is fine in this case as astropysQuantity are always cast to floats.

@rschwartz70
Copy link

rschwartz70 commented Sep 18, 2020 via email

@ehsteve
Copy link
Owner

ehsteve commented Jan 7, 2023

This was fixed.

@ehsteve ehsteve closed this as completed Jan 7, 2023
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

No branches or pull requests

3 participants