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

Clarification of tdwidth in find_doppler, get rid of FITS terminology E.g. NAXIS1 #98

Open
telegraphic opened this issue Aug 11, 2020 · 4 comments

Comments

@telegraphic
Copy link
Collaborator

tdwidth is a value used in the code that is unclear. NAXIS1 is not required as a parameter in general, should use a descriptive keyword instead of a FITS keyword.

In data_handler, there is this code:

        #EE For now I'm not using a shoulder. This is ok as long as
        ##  I'm analyzing each coarse channel individually.
        #EE In general this parameter is an integer (even number).
        #This gives two regions, each of n*steps, around spectra[i]
        self.shoulder_size = 0
        self.tdwidth = self.fftlen + self.shoulder_size * self.tsteps

Unclear why not just using FFT length.

@telegraphic telegraphic self-assigned this Aug 12, 2020
@texadactyl
Copy link
Contributor

@telegraphic

Does this comment suggest something to you?

For now I'm not using a shoulder. This is ok as long as I'm analyzing each coarse channel individually.

Does the shoulder concept come into play when "averaging across channels for large drift rates" (issue #76)?

Agreed about the use of NAXISn variables.

@texadactyl
Copy link
Contributor

texadactyl commented Feb 28, 2021

@telegraphic

I've looked at find_doppler.py, data_handler.py, and file_writer.py quite a bit lately. It looks like those FITS header fields are due to being porting from some other project some point back in time.

DATAH5 object instantiation, in effect, creates a second header and several data object fields. Several of these fields either (a) duplicate the filterbank header or (b) do nothing. Of course, some of them are important in search_coarse_channel processing.

header:  
    {'SOURCE': 'Voyager1',                  = source_name
    'MJD': 57650.78209490741,               = tstart
    'DEC': '12d10m58.8s',                   = src_dej
    'RA': '17h10m03.984s',                  = src_raj
    'DELTAF': -2.7939677238464355e-06,      = foff
    'DELTAT': 18.253611008,                 = tsamp
    'NAXIS1': 1048576,                      = nchans / n_coarse_chans = FFT length                                            
    'FCNTR': 8419.921873603016,             = frequency at the center of the list of channels
    'NAXIS': 2,                             NEVER USED
    'NAXIS2': 16,                           = n_ints_in_file
    'baryv': 0.0,                           at one time it was used but it is always 0
    'barya': 0.0,                           NEVER USED
    } 

# Currently needed in search_coarse_channel:
fftlen=1048576,                             = NAXIS1
tsteps_valid=16,                            = NAXIS2 = n_ints_in_file
obs_length=292.057776128,                   = tsteps_valid * DELTAT = n_ints_in_file * tsamp, units in seconds
shoulder_size=0                             ALWAYS 0
drift_rate_resolution=0.009566489757225039, = 1e6 * |DELTAF| / obs_length = 1e6 * |foff| / obs_length, units in Hz/s
tdwidth=1048576                             = fftlen + shoulder_size * tsteps = NAXIS1 + 0 = NAXIS1
                                            i.e. tdwidth ALWAYS = fftlen

tsteps vs tsteps_valid is interesting:

import math
import numpy as np
for tsteps_valid in range(1, 22):
    tsteps = int(math.pow(2, math.ceil(np.log2(math.floor(tsteps_valid)))))
    print(tsteps_valid, tsteps)

stdout:

1 1
2 2
3 4
4 4
5 8
6 8
7 8
8 8
9 16
10 16
11 16
12 16
13 16
14 16
15 16
16 16
17 32
18 32
19 32
20 32
21 32

@texadactyl texadactyl changed the title Clarification of tdwidth in find_doppler, get rid of NAXIS1 Clarification of tdwidth in find_doppler, get rid of FITS terminology E.g. NAXIS1 Jun 26, 2021
@texadactyl
Copy link
Contributor

texadactyl commented Jun 28, 2021

Having looked in this code recently, this will be a somewhat tedious and methodical process.
The data_handler.py actually created a 2nd header object and the source code language is not helpful.

It seems like the original author wanted to handle FITS input files as well as .h5 and .fil files.
It would have been better to plan for a blimpy "FITS to HDF5" utility as a front-end process, IMO.

@telegraphic ?

@texadactyl
Copy link
Contributor

texadactyl commented Sep 14, 2021

@telegraphic

I believe that the original code author was attacking issue #76 in a different way. If this is true, then the shoulder_size should have been a parameter. However,I suspect that this approach was 1/2-baked.

No excuses for the use of FITS metadata labels! Headers upon headers, ugh.

Yesterday, I made another pass at cleanup (issue #274). I am certain that more is needed. If you believe that a shoulder_size parameter adds no value, let me know and then I will:

  • Change all "tdwidth" references to "fftlen".
  • Note that fftlen is computed to be NFPC = the number of fine channels per coarse channel in the data handler.
  • Remove "tdwidth" from source code.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants