Repository navigation
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## dev-backend-mixpol #335 +/- ##
======================================================
+ Coverage 49.17% 49.40% +0.22%
======================================================
Files 54 54
Lines 27241 27266 +25
Branches 4662 4668 +6
======================================================
+ Hits 13397 13471 +74
+ Misses 12324 12262 -62
- Partials 1520 1533 +13 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Each site's gain and D-term table now takes its dtype from the site's feed_type in the Caltable constructor, with a zero-copy view like Obsdata uses for legacy data. This replaces the const_def helpers, so writers keep writing DTCAL. A site's D-term table is copied into the caltable's tarr as its time average, and applycal's tarr check skips the D-term columns.
This branch has not been deployed
This file contains hidden or 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
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.
What and why
Every site's gain and D-term tables were typed circular, whatever the station's feeds, so a linear station's X/Y gains were stored under
rscale/lscale.Caltablenow types each site fromtarr['feed_type']:Internally the module reads and writes the generic
p1scale/p2scaleandd_p1/d_p2names, soapplycal,merge,pad_scans,scan_avg,enforce_positiveandinvert_gainsare one code path for both bases.DTCAL_LIN/DTDTERM_LINnow survive save/load; the file format is unchanged, the basis rides onarray.txt.make_jonestypes its simulated cal table per station instead of always circular.Pure refactor, no new capability: this is the step before
applycallearns to apply gains on any basis (next PR). Circular data is byte-identical throughout.Design decisions worth discussing
ehc.caltable_dtypes(feed_type)is the singlefeed_type -> (DTCAL_*, DTDTERM_*)map.'rl'and'xy'only. Hybrid feeds raise likefield_rotation_matrixdoes;'??'says to declare the feeds;'lr'/'yx'now raise at construction (before, they were silently typed circular, which labelled p1 as R).(time, lscale, rscale)table used to come out with its feeds swapped. Tables with names that can't be placed in a slot raise instead of being stored and failing later.'xy'station is relabelled silently, because every solver still writesehc.DTCALwhatever the basis, so r/l names carry no information. The one direction that warns is x/y names under an'rl'station, since only basis-aware code writes those.plot_gains/plot_compare_gainspolacceptsR|L|X|Y|p1|p2|both, case-insensitive. A physical name from the wrong basis raises up front. The default is'p1', which is'R'on a circular site.mergerefuses the same site in two bases; disjoint site lists in different bases (ALMA in LIN, the rest in CIRC) merge as before.Bugs found along the way
plot_gains(pol='both')passed the validator and then hit an unset local; it has never workedplot_gainsboundtmins = tmaxes = gmins = gmaxes = []to one list, so the y-range was computed from the times as well as the gainsplot_compare_gainswith anypolother than'R'/'L'hit an unset localload_caltable's fallback filename was missing a path separatorsummary_plots.imgsum,modeling_utils.caltable_to_gainsand the interactive dashboard readrscale/lscaledirectly, which a linear station no longer has; they now use the generic namesDeferred
applycalstill forcesswitch_polrep('circ'); gains on any basis is the next PR,apply_dterms=Truethe one afterself_cal,network_cal,polgains_cal,modeling_utils) still writeehc.DTCAL; the constructor relabels, so nothing breaks, but they should write the site's own dtype eventuallymake_joneskeeps itsgainR/gainLvariable names: the surrounding gain model is circular-specific (gain_RLratio,rlgaincal), so only the carrier variables would have been renamedHow to test
142 + 231 + 95 pass. Sections 1-11 of
test_caltable.pyare the no-regression contract from #331; the one change there istest_pad_scans_keeps_linear_gain_dtype, whose station is now genuinely'xy'rather than a LIN table on an'rl'site, which dispatch makes self-contradictory. Section 12 is new.test_mixedpol.pygains themake_jonescal-table checks; they were confirmed to fail with the writer fix reverted.Part of #212.