-
Notifications
You must be signed in to change notification settings - Fork 22
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
Change default pixel width of DESI mocks to 0.8A #532
Comments
I wouldn't be surprised if 0.8 changed again, but we might as well be consistent in the meantime (including the exact phasing of the 0.8 grid). I've added this to the 20.5 project to keep it on our radar for the next release. Contributions welcome. See desispec/bin/desi_proc lines 680-685 for current defaults (and yes, it is unfortunately hardcoded in a script; consider cleaning that up while implementing this so that desisim automatically uses whatever desispec is using as default). |
coming back to this issue. We fixed this for quickquasars mocks in #533 . It seemed everything was right, but the balfinder is crashing for this mocks due to the following error
It works well for a pixel width of 1 and 0.6, but not 0.8... |
The 3 cameras have overlapping wavelength coverage, and for coaddition to work (without re-sampling) it requires that the different grids are aligned, not just that they have the same step. e.g. OK:
but not OK:
Check your simulations to see that they have aligned wavelengths grids on the 3 cameras, not just that they all have 0.8 A steps. FWIW, this is the grid used in Andes, though in the future we expect to expand that a bit more to get those last few photons off the edge of the CCDs:
|
Yes, precisely the 'r' and 'z' bands are not aligned, and that rises the issue in #L225-L231. The modification in quickqusars was rather simple, just to propagate an argument to control the pixel width. So I guess the edges for the wavelength arrays are defined somewhere and we need to check they are aligned for the requested pixel width. I'll dig a bit more. Thanks @sbailey ! |
Hi @alxogm - Was this fixed in quickquasars? If so, should we close the issue? |
Hi @andreufont, it was not fully solved since we changed the pixel width to 0.8A, but the overlapping wavelength range of the cameras is not aligned, so coaddition using desispec routine fails. I think it is until now that analysis on P1D mocks are facing this issue again, we can try to fix it. |
Hi @alxogm - I think we should change this, but this probably means increasing by one the "quickquasar version". I mean, these should probably be desi-3.0-4 mocks and so on, just like we changed from desi-1 to desi-2 when we changed the storing of the continuum and resolution matrices. Are there other minor changes that we wanted to do, so that we could coordinate them all and put them in the same version update? |
Yes, I think there are some small improvements that we can add to the same version code. I'll make a list in another issue, or PR, so we keep track of it. |
We recently found out that the Lyman alpha mocks had pixels of width 1A, instead of 0.8A.
We think this might be related to line 619-620 in simexp.py, that sets by default the output pixel width to 1A, when the user doesn't provide any value for dwave_out.
desisim/py/desisim/simexp.py
Line 620 in 3ae76e4
How confident are we that the final pixel width will be 0.8A? If we are confident, should we change this default value?
The text was updated successfully, but these errors were encountered: