ACHMartin / seastar_project

4 stars 0 forks source link

Change name of "init_level1_dataset" #178

Closed ACHMartin closed 1 year ago

ACHMartin commented 1 year ago

in Oscar/level1

I feel the function name does not really do what it's name mean, perhaps change to: merge_beams

In addition, the merging and association to aft, fore and mid, shouldn't depend on there position when we call the function, but rather on a field in the dataset (cf Karlus email making the link between Aft, Fore, Mid and ProcessedDoppler)

DavidMcCann-NOC commented 1 year ago

Agreed, easy enough change

With regards to the association of beam direction for the Antenna dimension, while I agree that it would be more robust to organise these from variables I think its lower on the list currently.

I did just check Karlus' method to identify the antenna direction from the ProcessedDoppler arrays, and while it works for the fore and aft antennas it doesn't work (as it stands with his description) with the mid.

Karlus' description: if the average between Min and Max are positive the antenna is 33 . i.e...FOR if it is zero the antenna is Mid if negative the antenna is Aft.

I take that as np.mean( [ds.MinProcessedDoppler, ds.MaxProcessedDoppler] ) Which for our current test data gives: Fore : 3239.3179 Aft : -3321.3755 Mid : -31.26001

We could treat the difference in the mid as 'small' and so effectively zero but its technically not and I'd rather not put in a hardcoded fix right now

DavidMcCann-NOC commented 1 year ago

Closed with #184