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

Major re-work of correlator M&C interface #611

Merged
merged 11 commits into from
Jul 19, 2022

Conversation

bhazelton
Copy link
Member

@bhazelton bhazelton commented Jul 9, 2022

Description

Add the following tables:

  • array_signal_source: Array-wide information about the commanded signal source

    • time* (GPS second): time when the input source was changed
    • source (string): One of "antenna", "load", "noise", "digital_same_seed", "digital_different_seed"
  • correlator_component_event_time: When correlator components had an event (related to normal observing)

    • component_event* (string): One of "f_engine_sync", "x_engine_integration" (in this case it's calculated from the Mcount, "catcher_start", "catcher_stop". (Note: "x_engine_integration" is not yet supported because the value for it is not in redis yet).
    • time* (GPS second): start time for this component (Note: a float, not an int)

Remove the following tables:

  • correlator_control_state
  • correlator_control_command
  • correlator_take_data_arguments
  • correlator_config_command

Motivation and Context

First part of work outlined in #606, covering the tables that have the required info in redis now.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • Schema change (any change to the SQL tables)
  • New feature without schema change (non-breaking change which adds functionality)
  • Change associated with a change in redis structure
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Version change
  • Build or continuous integration change
  • Other

Checklist:

Schema change:

  • My code follows the code style of this project.
  • I have added or updated the docstrings associated with my feature using the numpy docstring format.
  • I have created an alembic version file to produce the schema change.
  • I have tested looping upgrading and downgrading the alembic version and tests pass consistently.
  • I understand the updates required onsite (detailed in the readme) and I will make those
    changes when this is merged.
  • I have updated the schema documentation document with my changes (under docs/mc_definition.tex)
  • I have added tests to cover my new feature.
  • Unit tests pass on site (This is a critical check, CI can differ from site).
  • I have updated the CHANGELOG.

Breaking change checklist:

  • My code follows the code style of this project.
  • I have updated the docstrings associated with my change using the numpy docstring format.
  • I have detailed any changes required to other HERA repos above and I will coordinate
    implementing the changes across all the repos.
  • I understand the updates required onsite (detailed in the readme) and I will make those
    changes when this is merged.
  • I have added tests to cover my changes.
  • Unit tests pass on site (This is a critical check, CI can differ from site).
  • I have updated the CHANGELOG.

@codecov
Copy link

codecov bot commented Jul 9, 2022

Codecov Report

Merging #611 (b438a9a) into main (740e647) will decrease coverage by 0.01%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main     #611      +/-   ##
==========================================
- Coverage   98.11%   98.10%   -0.02%     
==========================================
  Files          35       35              
  Lines        5090     5007      -83     
==========================================
- Hits         4994     4912      -82     
+ Misses         96       95       -1     
Impacted Files Coverage Δ
hera_mc/correlator.py 100.00% <100.00%> (+0.29%) ⬆️
hera_mc/mc_session.py 99.62% <100.00%> (-0.04%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 740e647...b438a9a. Read the comment docs.

@bhazelton bhazelton marked this pull request as draft July 9, 2022 00:40
@bhazelton bhazelton requested a review from dannyjacobs July 9, 2022 00:42
@bhazelton bhazelton requested a review from mkolopanis July 11, 2022 22:23
@bhazelton bhazelton marked this pull request as ready for review July 14, 2022 23:14
@bhazelton
Copy link
Member Author

The codecov project level check is failing because I removed a bunch of lines of code. But the patch check (which is the required one) is passing and the full codecov report shows that all the new lines are covered and that number of uncovered lines actually decreased.

@mkolopanis
Copy link
Member

Thanks for updating this. I've looked through and from a dashboard approach I think we will be good with all this. I'll leave the rubber stamping for @dannyjacobs

Copy link
Contributor

@dannyjacobs dannyjacobs left a comment

Choose a reason for hiding this comment

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

Looked through changes one more time. All seem consistent and ok.

@dannyjacobs
Copy link
Contributor

Rubber stamp.

@dannyjacobs dannyjacobs merged commit d93ad69 into main Jul 19, 2022
@dannyjacobs dannyjacobs deleted the correlator_interface_rework branch July 19, 2022 17:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants