-
Notifications
You must be signed in to change notification settings - Fork 1
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
tickets/PREOPS-4640: Support simultaneus viewing of multiple simulations in schedview-prenight #106
Conversation
…st mean absolute deviation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have to say -- I haven't taken the time to properly leisurely read the maths and investigate the plotting results. I am glad there is a unit test which should catch basic problems with the matches. It might be good if one of the sequences was such that a field never showed up at all (I can't quite tell if that happens or not), and then checked that the final result was appropriate.
I'm going to go over and look at the notebook that results, and I figure you can update here if needed.
|
||
|
||
def often_repeated_fields(visits: pd.DataFrame, min_counts: int = 4): | ||
"""Find often repeated fields in a table of visits. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you make some comment in the docstring or in the code about what this is for? Why four visits to a given field for a "often visited field"?
In operations sims, four visits to a field would be something that was either a DDF, a near-sun twilight microsurvey visit, or a field which was caught during twilight and again during the night .. we shouldn't generally be covering other fields more than that. In auxtel sims now, four visits to a field is not uncommon.
Between these two use-cases, I'm confused what this is for.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, my intention was to exclude most WFD fields, but include fields where we spend a lot of time.
The idea is to provide a single table for people interested in specific fields without producing something too long to be convenient. That is, I expect that it will be much more common for a user to want to know "how many times are we likely we to observe this specific DDF field tonight" than it is for a user to want to know about any specific WFD field.
We might want something to do something similar for larger areas of WFD coverage, and I have a few ideas, but none I'm really happy with. This is something to be added later.
---------- | ||
visits : `pd.DataFrame` | ||
A table that must include both the columns listed in | ||
``sim_identifier_column`` and ``visit_spec_columns`` (below). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd typically call "visits" the visits from one simulation, so it may be worthwhile to add here something like
"This data frame holds the visits read from multiple simulations, concatenated together."
No description provided.