-
Notifications
You must be signed in to change notification settings - Fork 4
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
Frequency Bands can be defined in terms of band edges #150
Comments
Carve a place for assignment of frequency bands via a list of band_edges. [Issue(s): #150]
Band edges would normally be created with some knowledge of:
Band edge selection should be done via a UI that shows the user where the FCs are located on the frequency axis, and allows binning with a selection tool, or according to a standard (log-spaced) method of generating a frequency axis. Note that such methods often yield some empty bands at lower frequencies due to the initial slow growth of the geometric series of band edges and the sparseness of the FCs. How to handle this not 100% clear yet. We will not allow the DC component into the analysis. Setting the lower bound of the interval to 0 should be treated as setting it to be df/2. In any case, assuming you had a set of reasonable band edges that you pass in, would we want to "tidy them up" to use df/2 edges? I think yes, provided we were not going to change decimation factor (which would change sample_rate, and thus df), or window length, which would change N and thus df. This suggests a flow:
Next step
It turns out that while trying to do this, we are going to have to support more kwargs in config creator. This means it is a good idea to review the two methods of ConfigCreator:
At a minimum, these methods could be consolidated by making them both instances of a common _create method, but after studying the code, a better solution is:
OK. After much tidying, am able to pass the band edges into ConfigCreator in the correct place. Here is the rub: A. Force the user to use canonical edges, and thus alter the edges they provide This makes me want to define a parameter band_edge_definition = ["exact", "canonical"] Note that in either case, I need to identify the in-band fc harmonic indices within each user-defined-band, whether they map to the canonical band or not. Knowledge of the fc harmonic indices depends on knowing sample_rate and samples_per_window. OK, basic functionality has been shown. Things to do:
Also, the emtf_band_setup file is independent of the frequency values, although the frequencies are well defined in Processing class. If I define the frequencies at each decimation level at the start of the config creation, then the frequency bounds can be added to Bands object.
|
Cleanup, towards issue#150. Also reduced to only one test (py3.8) [Issue(s): #150]
-Fix emtf_band_setup_files to import from their own __init__.py -Add _band_specification_style attr to ConfigCreator -add fftfreqs method to decimation_level -add method (assign_bands) to processing.py to allow (prototype) band specifcati ons -correct some definitions in band.json [Issue(s): #150]
Added a simple test that creates bands from manually accessing the band edges that are assocaited with the emtf band default file. [Issue(s): #150]
Now, even if band definition is from an emtf band setup file, band edges are defined and used. -Deprecated read_emtf_bands method in processing.py -added compute_band_edges method to EMTF band setup file -deprecated from_emtf_band_setup & from_emtf_band_df in frequency_band.py -also, cleaned up calls to fftfreqs, replacing with get_fft_harmonics where appropriate [Issue(s): #150]
-modified frequency_bands_obj() method of decimation_level.py to remove duplication of band_edges assignment -tried to remove all calls to deprecated from_emtf_band_df() method [Issue(s): #150]
-Add some properties to decimation_level and processing classes to make access of frequency_bands info easy from Processing objects -Use this change to update test_define_frequency_bands.py to access bands programatically [Issue(s): #150]
This is looking pretty good. Bands can be set from either an emtf band_setup_file or via a dictionary of band edges. In future we may want to address the addition of a band_edge_definition = ["exact", "canonical"] in the config. I put a note in issue #47 about this |
Added a band_specification_style to the processing config at the top level. The band_setup_file path is also written to the config is relevant [Issue(s): #150]
Streamlined some methods in frequency_bands marked as deprecated Tracked down last usage to a test for the matlab_z_file_reader Tuned up the test for the matlab_z_file_reader so that it has an assert statement [Issue(s): #150]
Also: -add a few docstrings from last commit -restore all three versions tests in prep for merge into main [Issue(s): #150]
This should be done before either #47 or #67. It is the general use case and needs to be made to work with emtf band setup configuration (the current default). Infact, we should also fix issue #134 first as well.
Once that is running, want to allow user to simply request a number of bands per decade or bands per octave as a default, normally done in log-space, i.e. band edges form a geometric series.
e.g. band_edges: 1-2, 2-4, 4-8, 8-16, ...
The text was updated successfully, but these errors were encountered: