astro-datalab / notebooks-latest

Default set of Data Lab notebooks, by DL team and contributed by users
BSD 3-Clause "New" or "Revised" License
60 stars 48 forks source link

Add new GMOS Pyraf data reduction NB #163

Closed rnikutta closed 1 year ago

rnikutta commented 1 year ago

Data reduction example NB to showcase usage if the Gemini IRAF kernel at DL. NB contributed by Vini P and Brian M from US-NGO. @jacquesalice can you please review it?

jacquesalice commented 1 year ago

@rnikutta will do! Should I leave my comments in this PR or email Brian and Vini directly?

rnikutta commented 1 year ago

@jacquesalice You can make the comments right here on GH, thanks,

jacquesalice commented 1 year ago

@vmplacco @rnikutta I'm having an issue with the cell that uses bash and wget to download the FITS files from the Gemini archive. I'm running the notebook on the Data Lab notebook server under in this directory: ~/notebooks_20230123_193855/04_HowTos/DataReduction/PyrafGMOS/. But when I run the cell to download the files, it saves them in my ~/ directory and the rest of the notebook gives me errors like:

ERROR - GSREDUCE: Input image : N20190302S0248.fits does not exist
ERROR - GSREDUCE: Exiting with 1 errors

ERROR - GSREDUCE: Program execution failed with 1 errors

GSREDUCE done
vmplacco commented 1 year ago

Hi Alice,

Yes, it assumes that you're running on your home directory. If you do:

import os os.chdir(~/notebooks_20230123_193855/04_HowTos/DataReduction/PyrafGMOS/)

would it work?

Best, Vini

On Tue, Apr 18, 2023 at 10:48 AM Alice Jacques @.***> wrote:

@vmplacco https://github.com/vmplacco @rnikutta https://github.com/rnikutta I'm having an issue with the cell that uses bash and wget to download the FITS files from the Gemini archive. I'm running the notebook on the Data Lab notebook server under in this directory: ~/notebooks_20230123_193855/04_HowTos/DataReduction/PyrafGMOS/. But when I run the cell to download the files, it saves them in my ~/ directory and the rest of the notebook gives me errors like:

ERROR - GSREDUCE: Input image : N20190302S0248.fits does not exist ERROR - GSREDUCE: Exiting with 1 errors

ERROR - GSREDUCE: Program execution failed with 1 errors

GSREDUCE done

— Reply to this email directly, view it on GitHub https://github.com/astro-datalab/notebooks-latest/pull/163#issuecomment-1513572417, or unsubscribe https://github.com/notifications/unsubscribe-auth/ADDPMU5B3IH3JD3B75A6MSLXB3HWBANCNFSM6AAAAAAXC4F32I . You are receiving this because you were mentioned.Message ID: @.***>

jacquesalice commented 1 year ago

@vmplacco ah yes! That worked when I did:

import os
os.chdir('/home/jail/dlusers/ajacques/notebooks_20230123_193855/04_HowTos/DataReduction/PyrafGMOS/')

Or even:

import os
os.chdir('./')

I'll review the rest of the notebook, but this is something that will need to be added in case other users are also running on their home directory (knowingly or unknowingly).

vmplacco commented 1 year ago

Great, thanks!

Yes, we can add an extra cell there.

Best, Vini

On Tue, Apr 18, 2023 at 11:01 AM Alice Jacques @.***> wrote:

@vmplacco https://github.com/vmplacco ah yes! That worked when I did:

import os os.chdir('/home/jail/dlusers/ajacques/notebooks_20230123_193855/04_HowTos/DataReduction/PyrafGMOS/')

Or even:

import os os.chdir('./')

I'll review the rest of the notebook, but this is something that will need to be added in case other users are running on their home directory.

— Reply to this email directly, view it on GitHub https://github.com/astro-datalab/notebooks-latest/pull/163#issuecomment-1513586545, or unsubscribe https://github.com/notifications/unsubscribe-auth/ADDPMU6N2MXDPVBJWJH5KALXB3JGPANCNFSM6AAAAAAXC4F32I . You are receiving this because you were mentioned.Message ID: @.***>

rnikutta commented 1 year ago

So, it should work in the directory in which the NB is. Mike made a change to the kernel specifically for that.

@mjfitzpatrick , can you please work with Alice to identify what's causing the regression?

Thanks, Robert

jacquesalice commented 1 year ago

@bmerino95 @vmplacco my other comments:

  1. Can you add a bit more description about what is happening in each cell? For instance, in the Basic Reduction part what do the parameters being passed to gsreduce and gsflat represent (i.e. fl_flat, fl_gmosaic, fl_fixpix, etc)?
  2. In the same cell, I can see that the first 3 inimages are the files that were downloaded to my directory in the previous cell, but where do inflats="gsN20190302S0250" and inimages="gsN20190302S0248" come from?
  3. Again in this same section, you should inform the user that there will be a few dozen FITS files and temporary files created/removed from running this cell.
  4. In the "Display fully reduced spectrum" section, can you briefly explain how you are calculating the wavelength values from CD1_1 and CRVAL1?
  5. In the same section, change data = iraf_out[2].data to flux = iraf_out[2].data for clarity on what the variable contains.

Thanks! :)

vmplacco commented 1 year ago

Thanks @jacquesalice! I should be able to work on this on Thursday (there is an AURA meeting tomorrow and I am helping out with a NOIRLab Open House) and send it back for a final review.

vmplacco commented 1 year ago

Great, thanks! The DRAGONS one is coming soon...

On Thu, Apr 20, 2023 at 2:36 PM Alice Jacques @.***> wrote:

@.**** approved this pull request.

Looks great @vmplacco https://github.com/vmplacco thank you! I'm approving the PR but I think @rnikutta https://github.com/rnikutta wanted to take a look as well before we merge into production.

— Reply to this email directly, view it on GitHub https://github.com/astro-datalab/notebooks-latest/pull/163#pullrequestreview-1394798157, or unsubscribe https://github.com/notifications/unsubscribe-auth/ADDPMU624F3VKXO4J542Y2DXCGT5TANCNFSM6AAAAAAXC4F32I . You are receiving this because you were mentioned.Message ID: @.***>

rnikutta commented 1 year ago

Thank you @vmplacco and @jacquesalice . I only cleared the outputs, and will now merge. (We most often leave the output in a NB, but these ones here are very long).