Skip to content
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

Support realtime 2023a data format (SplitInIcePulses) #244

Merged
merged 65 commits into from
Jan 8, 2024

Conversation

mlincett
Copy link
Collaborator

@mlincett mlincett commented Nov 30, 2023

With these changes, we set the name of the base pulse series to SkyScanBasePulses we support the change in the input pulses name occurred with the realtime 2023a format.

As part of the prepare_frames, a pulses_name argument is used as the source to copy the pulses from. When pulses_name is None, we use a map from the realtime JSON "version" to the pulses name ("SplitInIcePulses" for 2023a) and "SplitUncleanedInIcePulses" for 2021a. The latter is also the default.

In general, an arbitrary pulses_name is supported in extra_json_message(). In the future, this can be used to read an arbitrary series of pulses instead of the mandated one.

Out of simplicity, I have decided to copy over the pulses to a new "static" name rather than use it as an attribute of the reco object.

@mlincett mlincett marked this pull request as draft November 30, 2023 17:59
@tianluyuan
Copy link
Contributor

@mlincett I merged in the latest updates from main and updated the baseGCD url to point to prod-exe. Now that the Dockerfile is also pulling in the baseGCDs from prod-exe this might be redundant, but perhaps it's ok for now? Otherwise I think this is good to go and I'll go ahead and mark it as ready for review.

Was there anything else you were working on?

@tianluyuan tianluyuan marked this pull request as ready for review January 5, 2024 22:42
@mlincett
Copy link
Collaborator Author

mlincett commented Jan 5, 2024

@mlincett I merged in the latest updates from main and updated the baseGCD url to point to prod-exe. Now that the Dockerfile is also pulling in the baseGCDs from prod-exe this might be redundant, but perhaps it's ok for now? Otherwise I think this is good to go and I'll go ahead and mark it as ready for review.

Was there anything else you were working on?

Thank you for picking this up! I believe it should be good to go.

(We will not have single-pixel tests for the new format but I don't see that as strictly necessary for now.)

Initially, I meant to rename the input pulse series name of the scanner to something neutral and different from "SplitUncleanedInIcePulses", but this would require replacing all the single-pixel test data (or finding some workaround). I guess we can ignore that for now.

Copy link
Member

@ric-evans ric-evans left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A few questions and requests

skymap_scanner/server/start_scan.py Outdated Show resolved Hide resolved
skymap_scanner/utils/extract_json_message.py Outdated Show resolved Hide resolved
skymap_scanner/utils/extract_json_message.py Show resolved Hide resolved
@tianluyuan tianluyuan merged commit 4dba8f2 into main Jan 8, 2024
37 of 38 checks passed
@tianluyuan tianluyuan deleted the realtime-2023a branch January 8, 2024 20:33
@tianluyuan tianluyuan mentioned this pull request Jan 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants