sbird/fake_spectra

Spectra.dvbin has different meaning when loading from savefile

Closed this issue · 2 comments

When reloading from a snapshot, Spectra.dvbin is set by the user-provided option "res" (line 200 in spectra.py).

However, when loading from a saved spectra file, it is set to the actual width of the pixel, i.e., vmax / nbins (line 197).

Unless the option "res" is carefully chosen to be an exact fraction of vmax for each redshift, the two quantities will not match.

I'd like to suggest to modify the code such that dvbin is always set to vmax / nbins, the actual bin size, even if that doesn't quite match what the user asked for.

I could submit a PR if you agreed.

sbird commented

I completely agree with this! Thanks for noticing and please send a PR. I think this is a python3 porting bug, because it used to do integer rounding division in python2.

Hi @sbird - digging a bit further into the code, I think I'd like to remove the whole try / except block around lines 191 - 202.

There are three possibles modes:
A) you are reloading from snapshot, in which case self.nbins is not set
B) you are loading from savefile, and you have requested res != self.nbins
C) you are loading from savefile, and you have requested res == self.nbins

In the current version of the code:
A) line 193 will automatically raise an AttributeError, since self.nbins is not define, and move to line 198
B) if statement evaluate to True, and line 194 raises an AttributeError, and move to line 198
C) if statement evaluate to False, move to 197, where we set self.dvbin = self.vmax / (1.*self.nbins)

Note that in the current code, if you ask for a value of "res" different than the one stored in savefile, the code will follow (B) and set dvbin = res , what does not really match what was read from the file!

I'll submit a PR that drops the try / except and prints a warning error if you specify a value of res that is not consistent with the one read from file.