Refactor bootstrapping code #27
Conversation
There was a problem hiding this comment.
In the "group_baselines" function: If "keep_remainder" is True, can we tack on each of the 'remainder' baselines onto a group? For example, if there are 52 baselines and 5 groups, instead of having 5 groups of 10 and a remainder group of 2, can we have 5 groups total, where 3 of the groups have 10 and 2 of them have 11?
|
Sure, I can make it do that. |
|
OK, I've addressed @carinacheng's comment, fixed some bugs, and added unit tests. |
* Implement test to check normalisation using Parseval's theorem (based on code by Aaron E-W) * Add functions to return power spectrum units and array of delays
|
I've now (hopefully) fixed the units of |
|
|
||
| # Randomize baseline order if requested | ||
| if randomize: | ||
| if seed is not None: random.seed(seed) |
There was a problem hiding this comment.
Would it make more sense to have the seed default to a time seed if it's "None"? When no seed is used, the answer ends up the same every time, and I think we want to avoid that.
| Bootstrap-sampled set of baselines (will include multiple instances of | ||
| some baselines). | ||
| """ | ||
| if seed is not None: random.seed(seed) |
| raise IndexError("No datasets have been added yet; cannot " | ||
| "calculate power spectrum units.") | ||
| else: | ||
| pspec_units = "(%s)^2 (ns)^-1" % self.dsets[0].vis_units |
There was a problem hiding this comment.
I don't see any other reference in the code to vis_units. Has this been implemented?
There was a problem hiding this comment.
self.dsets[0] is a UVData object. The UVData class has a standard property called vis_units, so I think this should be fine.
| Test baseline grouping behavior. | ||
| """ | ||
| # Generate example lists of baselines | ||
| bls1 = [(0,i) for i in range(1)] |
There was a problem hiding this comment.
The original pspecdata tutorial notebook has tuples with 3 elements. I think the code works fine with 3-element tuples (right?), but all of this should be clarified.
There was a problem hiding this comment.
Yes, I'll add a comment in the tests. This code just shuffles tuples or lists of tuples -- it doesn't care if they are valid 3-element baseline tuples or not. I think we don't need a validity check at this point either, as all of that will be done on passing the baselines into pspec()
hera_pspec/tests/test_pspecdata.py
Outdated
|
|
||
| def test_parseval(self): |
There was a problem hiding this comment.
I'm actually no longer sure that this is the right test. Let's talk about it in person.
|
|
||
| # Check that random seed works | ||
| g1 = boot.group_baselines(bls5, 3, randomize=True, seed=10) | ||
| g2 = boot.group_baselines(bls5, 3, randomize=True, seed=11) |
| """ | ||
| Group baselines together into equal-sized sets. | ||
|
|
||
| These groups can be passed into PSpecData.pspec(), where the corresponding |
There was a problem hiding this comment.
Can we have an explicit demonstration of this?
|
By agreement with Adrian, some of the requested changes (e.g. to documentation) can be made at a later date. We will merge this now to push forward the graduation of this code into |
Refactor the support functions for bootstrapping, and the way
PSpecDatahandles groups of baselines.