-
Notifications
You must be signed in to change notification settings - Fork 3
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
Add a recipe of estimateNoise #1
base: main
Are you sure you want to change the base?
Conversation
I'm reviewing this today. The first thing that will need to be corrected is the use of the Also related to the structure, we keep the tests "with the code". As such, the 'astrodata' test should be moved under Where can I get the I'll be digging deeper into the code today. It looks like a great start. |
igrins_instruments/igrins/init.py Is there a reason why there are two On line 10, the “fox” should be replaced with “IGRINS”. My mistake when I set up the template. -- igrins_instruments/igrins/adclass.py Looks good. We’ll use that for development. There might be a way to combine the The data_label descriptor for IGRINS-2 will have to use the DATALAB keyword eventually because IGRINS-2 will be a facility instrument and will have all the standard Gemini keyword values. At some point, the IGRINS-2 simulated or test data will have to also simulate the standard Gemini keywords instead of the IGRINS makeshift one. -- igrinsdr/igrins/igrins_pipeline/utils/image_combine.py The use of stsci.image will need to be replaced. STScI seems to have abandoned support. The package is only available for Python 3.7 and lower. DRAGONS v3.1+ will require Python 3.9+. (Also, note that it would be advisable to do the IGRINS-2 development with Python 3.9.) -- igrinsdr/igrins/igrins_pipeline/procedures/* I just checked the imports not the code. Looks okay. -- igrinsdr/igrins/lookups/timestamp_keywords.py Each primitive that you write needs a timestamp keyword. -- igrinsdr/igrins/procedures I’m curious, how is the “procedures” directory here different from the “igrins_pipeline/procedures”? -- igrinsdr/igrins/recipes/sq The recipes are not quite right and we will need to discuss this. More below. -- igrinsdr/igrins/recipes/sq/recipes_DARK.py The -- igrinsdr/igrins/recipes/sq/recipes_FLAT.py The This is in a FLAT recipe library, which means that the inputs are FLATs, yet it calls I will need to know more about what this recipe is producing and from what type of inputs. I suspect that these calculations need to be associated with a I had started making DRAGONS-compatible flow charts for IGRINS. We probably should sit down together and look over them and see how to make it all fit the DRAGONS way. -- igrinsdr/igrins/parameters_igrins.py All primitives must have a -- igrinsdr/igrins/primitives_igrins.py I would like to discuss the primitives that you have created. Some of them are probably not recommended or required. (eg.
Why are you using stream in -- General comment. I understand that the primary objective is to get something to work in DRAGONS with as little effort as possible, and some of the comments above do not necessarily require immediate action, they could cleaned up later. However, I think that we should sort out the recipes to make them DRAGONS-like. I’m more concerned about the recipes and the products (like where tables and solutions are stored) then the internal of the primitives. I propose that we meet to work together on DRAGONS flowcharts. Once those are in place, I think that you will have no problems fitting the igrins algorithms into primitives. What you have accomplished already with no support from me is quite impressive. |
…d in the gemini_instruments igrins via direct keywords
…rprised it worked at all before.
The PR adds a new recipe of estimateNoise, which will work with flat off images. While the recipe is more-or-less for QA purpose, it is adequate to test various aspects of the DRAGONS recipe system. It also includes updated AstroData class for IGRINS. For now, the recipe expects idata from IGRINS, i.e., I assume there are separate files for H and K detectors. I understand that IGRINS2 will have both H and K in a single file, and it should be straight forward to adopt it.
You can install it via 'pip install' command. Currently the 'pyproject.toml' file does not contain any dependency for dragons. So, you need to manually install dragons before installing igrinsdr.
Once installed, you should be able to do