deeplycloudy / lmatools

Python code for working with VHF Lightning Mapping Array data
BSD 2-Clause "Simplified" License
22 stars 23 forks source link

New Flash Sorting and Gridding Products #16

Closed Vsalinas91 closed 7 years ago

Vsalinas91 commented 7 years ago

Added total and specific energy, and flash size standard deviation fields to both the flash-sorted and gridded files.

Flash sorted files now write out volume as well.

Power-Law fit has also been updated for Cm^-3.

deeplycloudy commented 7 years ago

@Vsalinas91 I look a look at the pull request and have a few notes. This comment contains some of the simpler todos. I'll write separate comments for some of the bigger items.

deeplycloudy commented 7 years ago

In lasso/energy_stats.py, when calculating the energy statistics for the flash population, you added two calculations 248-254, lasso/energy_stats.py, which give the specific energy and total energy. You take as representative values

result['specific_energy'] = eng_mean eng_number result['total_energy'] = teng_mean teng_number

Why did you do this instead of the simpler calculation

flashes['tot_energy'].sum() flashes['Energy'].sum()

The latter calculation will be exact; the first will not unless the population of flash energies is symmetric about the mean.

deeplycloudy commented 7 years ago

Beginning line 279 there is a long addition of new code. Previously, there was a loop that

  1. Grabbed a bundle of flashes, and their start and end times
  2. Sent those flashes to function that plotted the data and saved the spectra to a file. Please review this section of code to follow the previous pattern. This helps keep the plotting functions self-contained, and the function call signature makes it clear what data are being used to make a certain plot. It also keeps the cell-lasso-stats.py script shorter, and easier to understand the flow of data through it. The plot functions should be added to lmatools/flash_stats.py
Vsalinas91 commented 7 years ago

I'll try to update these changes here from the above comments.

For the first set:

Second set:

Third set:

Also, I had that weird permissions error issue which was solved by changing the permissions to all the files, specifically cell-lasso-stats.py. Because of this, there will be 30 changed files, some of which have no change at all to them. I guess git recognizes changes in permissions as a modification of a file or directory.

deeplycloudy commented 7 years ago

Thanks for the changes to the second and third sets. Those look good.

In the first set, the only thing that still remains are a few places where the diff shows that there were changes due to whitespace. It's harder to find the substantive changes with those in there, so please review the "Files Changed" tab one more time to get rid of those.

I see what you mean about those permission changes. That's really odd, and there's no reason those files should need to be executable. Can you remind me of what error you got before changing the permissions? I'd like to get rid of that if at all possible.

As you work on those two things, I'll try a test on some sample data locally to make sure everything runs for me.

deeplycloudy commented 7 years ago

I just tested the two scripts, and things ran with no problems.

I did not have to uncomment the h5_files and run the flash sort script a second time. For whatever reason the h5_filenames are returned correctly, and the gridding function picks them up and runs with them. I wonder if it's because I usually give a path like '/data/whatever/' instead of './whatever' when I give the path to the outfiles.

One minor remaining bug, which has been there a while: there is a missing comma at the end of the second to last line of flash_sort_and_grid.py, after the datettime.

Vsalinas91 commented 7 years ago

It also worked for me too without having to comment out the h5_files. Why? -- Because I had manually renamed the h5_files directory name to be created when processing the data originally. Changed it, ran it again and realized that it works just fine.