Closed leungcalvin closed 3 years ago
Hi @leungcalvin
This is looking good, and I don't think it needs a huge amount more work before it's ready to merge. Here are some initial comments:
- Please rebase this into a set of logical commits with messages following our dev guidelines (https://github.com/chime-experiment/Pipeline/blob/master/CONTRIBUTING.md#rules). I'll leave you to figure out exactly what to do, but you need at least these changes in separate commits: changing the default location for CHIME; adding support for changing the observatory to the delay/fringestop routines; and adding support for TONE.
- I'm a bit averse to having a dump of all the antennas in directly, just because it dumps in a lot of redundant info. Can you wrap it up into something which programmatically generates them?
- There's a lot of commented out lines (e.g. old params, print statements, etc). Please tidy those up.
- Please fix up the merge conflicts, I think the CI won't run until you have.
Hi Richard,
I've addressed these comments in a series of logically-chunked commits.
Oh also @leungcalvin when you're done, please rebase this such that only the actual relevant commits are left. I think that should be the three feat(...)
ones and the test
one.
Refactored ephemeris, added best known CHIME phase center position, and included some modifications for GBO/TONE outrigger. Currently the TONEAntenna objects are hard coded. The fix is entangled with the long term solution to
chimedb_config
andget_correlator_inputs
, but if we're hard coding the array coordinates I see no problem with hard coding the small number of manageable TONEAntennas while we figure out that problem.