Karr/update interface #50
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
It seems you branched from a very old version of the repository. See perhaps you could try to rebase the branch, or merge main into it? |
|
A small note about |
MJDOBS Determination -------------------- The date of observation is now determined as follows - if --sequence is set to a date in the format yyyy-mm-dd hh:mm:ss the code will start from that date and increment based on observing time of the previous call to simulate. - if --sequence = 1, the observations will start from the date given in the first entry in the YAML file - if --sequence is not set, the observations will take the date directly from the YAML file. If an entry other than the first one doesn't have mjdobs in the YAML entry, the observation time will be incremented as above. mjdObs is no longer an expandable. Instead, there is a new paramter, nObs, in the YAML file. nObs observations of each combination in the entry will be generated with sequential timestamps, based either on the previous exposure or the mjdObs value depending on the --sequence options. e.g. > CALIBRATION FILES ----------------- If the option --doCalib is set to an integer > 0, the script will determine what combination of darks and flats need to be observed as part of the sequence of observations, and will execute them at the end of the observations given in the YAML file. At the moment, DARK,WCUOFF files are not included; they may be added later. The value of doCalib, if > 0, determines the number of each type to be execute. DEBUGGING --------- Some more informative output is written to the screen; the values of the run time options, and the parameters of each call to simulate(). If the command line option --testRun is set, the script will be run, the YAML file parsed, and mdjObs calculated, but simulate() will not be called. This is intended for debugging and checking input. Added a routine checking the YAML file for valid input. It will check that mode, prefix, properties, dit, ndit, filter_name, catg, tech, type, nObs exist, check that ndit and nObs are positive integers, dit a positive number, and for valid values of mode, catg, tech, type (as imported from simulationDefinitions). If invalid input is detected, an error message is written and the code aborts. Currently filter_name, ndfilter_name or source are not checked. PREFIX ------ This was set before, but to highlight the change. Each YAML entry now has a prefix field in addition to mode, because the labels for each recipe need to be unique, while the prefix to the output file does not. prefix is solely used to label the entries; mode is used for generating file names and setting DO.CATG.
- separated downloads into downloads.py, to be run after installing
and if the packages need updating.
- automatically call updateHeaders at the end of the script
- separated simulationDefinitions from sources
- changed the prefix parameter to do.catg, which is what it should be.
- significant rewrite of README
- updated installation / basic run instructions
- more extensive command line options section
- added section describing how to run your own simulations
Changed mjdobs to dateobs because it's not actually MJD updateHeaders.py won't be called if --testRun is set updated README to match
JenniferKarr
force-pushed
the
karr/updateInterface
branch
from
d9a9c2a to
48f2816
Compare
|
I've rebased to master, ran the code locally, and tidied the recipes.yaml afterwards. The automated tests need to be updated to run downloads.py before run_recipes.py, to download the necessary packages. |
hugobuddel
requested changes
Sep 13, 2024
There was a problem hiding this comment.
If I run ./run_recipes.py, it will give several warnings, but not actually simulate anything it seems:
$ ./run_recipes.py
Starting Simulations
input YAML = [...]/METIS_Simulations/ESO/recipes.yaml, output directory = [...]/METIS_Simulations/ESO/output
Observation dates will be taken from YAML file if given
Automatically generated darks and flats 0
Small output option False
Recipe LM_SLITLOSSES_RAW1 has invalid TYPE of SLITLOSS)
Recipe LM_SLITLOSSES_RAW2 has invalid TYPE of SLITLOSS)
Recipe LM_SLITLOSSES_RAW3 has invalid TYPE of SLITLOSS)
Recipe LM_SLITLOSSES_RAW4 has invalid TYPE of SLITLOSS)
Recipe LM_SLITLOSSES_RAW5 has invalid TYPE of SLITLOSS)
Recipe LM_SLITLOSSES_RAW6 has invalid TYPE of SLITLOSS)
Recipe LM_SLITLOSSES_RAW7 has invalid TYPE of SLITLOSS)
Recipe LM_SLITLOSSES_RAW8 has invalid TYPE of SLITLOSS)
Recipe LM_SLITLOSSES_RAW9 has invalid TYPE of SLITLOSS)
Recipe LM_SLITLOSSES_RAW10 has invalid TYPE of SLITLOSS)
Recipe LM_SLITLOSSES_RAW11 has invalid TYPE of SLITLOSS)
Recipe N_LSS_SLITLOSSES_RAW1 has invalid TYPE of SLITLOSS)
Recipe N_LSS_SLITLOSSES_RAW2 has invalid TYPE of SLITLOSS)
Recipe N_LSS_SLITLOSSES_RAW3 has invalid TYPE of SLITLOSS)
Recipe N_LSS_SLITLOSSES_RAW4 has invalid TYPE of SLITLOSS)
Recipe N_LSS_SLITLOSSES_RAW5 has invalid TYPE of SLITLOSS)
Recipe N_LSS_SLITLOSSES_RAW6 has invalid TYPE of SLITLOSS)
Recipe N_LSS_SLITLOSSES_RAW7 has invalid TYPE of SLITLOSS)
Recipe N_LSS_SLITLOSSES_RAW8 has invalid TYPE of SLITLOSS)
Recipe N_LSS_SLITLOSSES_RAW9 has invalid TYPE of SLITLOSS)
Recipe N_LSS_SLITLOSSES_RAW10 has invalid TYPE of SLITLOSS)
Recipe N_LSS_SLITLOSSES_RAW11 has invalid TYPE of SLITLOSS)
[...]/METIS_Simulations/ESO/recipes.yaml [...]/METIS_Simulations/ESO/output False None
Recipe LM_SLITLOSSES_RAW1 has invalid TYPE of SLITLOSS)
Recipe LM_SLITLOSSES_RAW2 has invalid TYPE of SLITLOSS)
Recipe LM_SLITLOSSES_RAW3 has invalid TYPE of SLITLOSS)
Recipe LM_SLITLOSSES_RAW4 has invalid TYPE of SLITLOSS)
Recipe LM_SLITLOSSES_RAW5 has invalid TYPE of SLITLOSS)
Recipe LM_SLITLOSSES_RAW6 has invalid TYPE of SLITLOSS)
Recipe LM_SLITLOSSES_RAW7 has invalid TYPE of SLITLOSS)
Recipe LM_SLITLOSSES_RAW8 has invalid TYPE of SLITLOSS)
Recipe LM_SLITLOSSES_RAW9 has invalid TYPE of SLITLOSS)
Recipe LM_SLITLOSSES_RAW10 has invalid TYPE of SLITLOSS)
Recipe LM_SLITLOSSES_RAW11 has invalid TYPE of SLITLOSS)
Recipe N_LSS_SLITLOSSES_RAW1 has invalid TYPE of SLITLOSS)
Recipe N_LSS_SLITLOSSES_RAW2 has invalid TYPE of SLITLOSS)
Recipe N_LSS_SLITLOSSES_RAW3 has invalid TYPE of SLITLOSS)
Recipe N_LSS_SLITLOSSES_RAW4 has invalid TYPE of SLITLOSS)
Recipe N_LSS_SLITLOSSES_RAW5 has invalid TYPE of SLITLOSS)
Recipe N_LSS_SLITLOSSES_RAW6 has invalid TYPE of SLITLOSS)
Recipe N_LSS_SLITLOSSES_RAW7 has invalid TYPE of SLITLOSS)
Recipe N_LSS_SLITLOSSES_RAW8 has invalid TYPE of SLITLOSS)
Recipe N_LSS_SLITLOSSES_RAW9 has invalid TYPE of SLITLOSS)
Recipe N_LSS_SLITLOSSES_RAW10 has invalid TYPE of SLITLOSS)
Recipe N_LSS_SLITLOSSES_RAW11 has invalid TYPE of SLITLOSS)
hugobuddel
reviewed
Sep 13, 2024
hugobuddel
reviewed
Sep 13, 2024
Thanks!
I interpreted this as that you were still going to update the tests before asking for a review. But I reviewed the code anyway. Could you please update the tests? I think we should have at least someone be able to replicate the simulation run, I couldn't yet do it. |
updated workflow
reworked handling of automatically generated flats/darks to use a list of dictionaries in the same form as that read from the YAML file, rather than hard to parse lists and sets. This includes the definition template dictionaries in simulationDefinitions.py.
|
I've addressed the various comments as written, with the exception of reworking the generation of the set of flats/darks to use a list of dictionaries rather than various impenetrable set/list combinations. This is easier to follow and more compact. To address two comments: The N band laser is available only during AIT, as per the DRLD. I kept use of do.catg, as we already use catg in the template definition, for the CALIB, etc. |
|
Great thanks. I'm running the simulations now! Note that this page (#50) shows all the unresolved comments in a nice overview, and just scrolling up shows that there are 5 still open. (I marked some as resolved.) These comments are mostly small ones, and can easily be resolved by pressing the 'Commit suggestion' button. But I don't want to press that button myself because doing so would change 'your' branch on github, which you might find annoying if it happens unexpectedly. Could you therefore go over these suggestions? |
hugobuddel
approved these changes
Sep 25, 2024
There was a problem hiding this comment.
Approved, since I can now run the code, and the tests run too. And of course it is a big improvement! I did not yet verify whether I get the same hashes though.
I would appreciate it though if you could still go through the small suggestions I made; it shouldn't take more than a few minutes.
Co-authored-by: Hugo Buddelmeijer <[email protected]>
Co-authored-by: Hugo Buddelmeijer <[email protected]>
Co-authored-by: Hugo Buddelmeijer <[email protected]>
Co-authored-by: Hugo Buddelmeijer <[email protected]>
Co-authored-by: Hugo Buddelmeijer <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Some fairly major updates for the simulations
Command Line
YAML keyworlds
Structure