takdg123/vtspy

Binsperdec not being passed through to fermi sed generation

Closed this issue · 4 comments

The following line:

o = self.gta.sed(self.target.name, outfile=outfile, bin_index=kwargs.pop("bin_index", 2.0), loge_bins=loge_bins, write_fits=True, write_npy=True, **kwargs)

sets the binning for the Fermi sed generation differently than is specified by the binsperdec provided in the config.yaml file.

This can most simply be resolved by replacing the loge_bins=loge_bins with loge_bins=self.gta.log_energies, which will pull the binning straight from the GTAnalysis object, which has been initialized with the Fermi portion of the config file. Beyond that, however, there is likely still some cleanup needed in the surrounding areas to remove extraneous code. Not sure if there needs to be edge cases accounted for in case of binsperdec missing from the config file.

I have already written the preliminary code that can fix this, but am waiting to submit a PR as it's built on top of the currently existing PR #6

I just merged PR #6.
Your suggestion is good, but then we will somehwat lose the control over nbins right?

I guess we use binsperdec as part of the dataset construction for the gammapy analisys, but not for the fermipy analysis where we rely more on nbins. Maybe something like this would solve?

        if kwargs.get("nbins"):
            self._energy_bins = MapAxis.from_bounds(energy_bounds[0].to(u.MeV).value,
                energy_bounds[1].to(u.MeV).value,
                nbin=kwargs.pop("nbins"),
                name="energy",
                interp="log", unit="MeV")

            loge_bins=np.log10(self._energy_bins.edges.value)
        else:

            loge_bins = self.gta.log_energies

I agree, think that seems like a reasonable solution