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

improve the way that the fft is performed on chirps #51

Merged
merged 19 commits into from
Oct 23, 2024

Conversation

jkingslake
Copy link
Member

@jkingslake jkingslake commented Oct 9, 2024

This changes the way the fft is performed on the chirps

Previous way

An fft is done on each chirp one-at-a-time as we load them in load.ChirpObject.FormProfile
For padding and rolling it did some manual indexing, which was difficult to understand and has caused one small bug (#48). This function was also not easily called from outside load.load_all, for example if you have previously-loaded chirp data you wanted to reprocess. It was also hard to change fft settings like the padding factor.

New way

This PR moves the fft functionality to utils.computeProfile, a separate function that is added to xarray.dataarrays as a bound method. It uses xarray to do the rolling, padding and fft. This has a number of advantages:

  • it can be called at a higher level during loading with load.load_all, so the whole dataset is fft'd at once (seems more elegant and appears to be faster by a factor ~2, very approximately)
  • it is easier to understand
  • it avoids the error in padding/rolling chirps leaves two erroneous zeros  #48
  • (major benefit) it allows options to be set during loading that effect the fft, for example the padding factor and the start and end frequencies (for cropping the chirps)
  • (major benefit) it allows the function to be called on existing chirp data, not only when it is loaded, but once it has previously been loaded and fft'd. In such a case, it can be useful to change settings (above point) and see how it effects the results without having to reload the chirps from the .dat files each time.

Usage

Usage is shown in note form in a new NB, notebooks/test_notes/custom_fft_implemention_testing.ipynb and with more explanation, but probably less comprehensively in notebooks/guides/usingXApRES.ipynb.

When loading data

When loading you use load.load_all in the same way as before

ds = fd.load_all(directory='../../data/sample/single_dat_file/')

you can also change settings for the fft by passing a dict as addProfileToDs_kwargs, for example,

ds = fd.load_all(directory = '../../data/sample/single_dat_file/', addProfileToDs_kwargs = {'crop_chirp_start': 0, 'crop_chirp_end': 0.5}

When you have pre-loaded data

To replace the profile variable in a previously-loaded dataset:

big = xa.load.load_zarr()
some = big.isel(time = slice(4000, 4100), chirp_num = 0).load()
new_ds = some.addProfileToDs()

or to just compute the profile:

profile_xa = some.isel(time = slice(0,10), attenuator_setting_pair = 0).chirp.computeProfile(max_range=1500, crop_chirp_start = 0.8)

Constants like K, B, f_1 and f_2, can be provided by the user. During loading of data with load.load_all constants from the header of the last loaded burst are used. In other cases default are used.

New functions

  • utils.computeProfile for computing profiles from an chirp dataarray
  • utils.addProfileToDs for computing profiles and adding them to a dataset. If the dataset already has a profile variable it is replaced
  • utils.default_constants defines default constants for the fft.

Summary

This PR adds a new (better) way to compute ffts of chirps which is cleaner and clearer, more flexible and easier to use outside the data-loading stage.

Merge pull request #45 from ldeo-glaciology/ongoing-work-jk-07-24
The NB contains a working function that implements all the things I was hoping to get working:
- fft of mautli-dimensional datarrays
- Georges's demean, detrend, drop bad chirps.
- reference array
- agrees with the code implemented in xapres currently (- the error in the padding described in an issue #48)
also adds an option to load all to use the legacy fft method and to uses a corrected padding procedure for comparison with the new methods.

custom_fft_implementation.ipynb tests this out, but needs more work to make it understanable.
@jkingslake
Copy link
Member Author

jkingslake commented Oct 9, 2024

todo:

  • add some tests of the new code

  • add information on how to run the new functions in usingApRES.ipynb

add a test of the new fft methods and compares the to the previous way of doing it and doing the same thing in different ways with the new methods, to make sure they all give the same answer.

Also edited test_file_selection_methods so that it loads from local, not a google bucket. Just to speed things up a bit.

All test run in 30s on my local machine.
this will need more testing. Currently I have not been using these features and by default they are turned off.
@jkingslake jkingslake marked this pull request as ready for review October 10, 2024 15:51
@jkingslake jkingslake marked this pull request as draft October 10, 2024 15:59
@jkingslake
Copy link
Member Author

@Elizabethcase @glugeorge, I put you down as reviewers on this PR. But, seeing as no one else is working on this repo right now, and so they dont rely on the code, I think I will merge this and deal with any corrections or improvements later.

@jkingslake
Copy link
Member Author

Actually, I have a few more edits to make, related to the chunking of the chirpa before the fft is performed.

The pass operation adds multiple chunks in the chirp_time dimension and the fft doesn't like that, so I need to correct that.

@jkingslake jkingslake marked this pull request as ready for review October 23, 2024 12:48
@jkingslake jkingslake merged commit 310cfc3 into master Oct 23, 2024
3 checks passed
@jkingslake jkingslake deleted the custom-fft-xarray branch October 23, 2024 15:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant