Skip to content

Conversation

@jwaiton
Copy link
Member

@jwaiton jwaiton commented Dec 11, 2024

This PR introduces the functionality of processing into MULE, with single channel decoding from waveDump 1 .dat files now being possible. Addresses #11 which will be resolved when this is complete.

The data is stored in h5 files with a storage path and name provided by the user. The h5 format matches that decoded in wavedump 2.

This PR rests on top of #10, and as such includes all the new bells and whistles. Config files, integration with the mule executable.

Things still needed

  • tests
  • refactor the code
  • remove the deprecated function

@jwaiton jwaiton mentioned this pull request Feb 12, 2025
@bpalmeiro bpalmeiro marked this pull request as ready for review February 18, 2025 18:56
@bpalmeiro bpalmeiro self-requested a review February 18, 2025 18:56
Copy link
Collaborator

@bpalmeiro bpalmeiro left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not real hehe

Copy link
Collaborator

@bpalmeiro bpalmeiro left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

request

@jwaiton jwaiton force-pushed the include-WD1-processing branch from aa6ef26 to c08aad7 Compare February 20, 2025 15:49
@jwaiton jwaiton mentioned this pull request Mar 18, 2025
@jwaiton
Copy link
Member Author

jwaiton commented Mar 18, 2025

Sorry for bloating PRs, but in this PR I'm including a smarter way to process data that is memory efficient. We now have a writer object that can writes individual waveforms to the h5 file.

I need to add forwards compatible functions for reading datasets lazily now, and make them group and dataset agnostic. This is attached to #31, and will be done in a different PR.

EDIT: To avoid this getting out of hand, I'll add tests for the readers and writers in a separate PR, or perhaps I'll extract them into a separate PR entirely (the smarter but possibly more complicated move git-wise).

EDIT 2 (29/03): The readers and writers are also a bit stupid, and waste a lot of time with IO operations (reopening datasets that should rest open at all times). I tried to think of a smart solution to keep them open but it gets complicated (or I'm tired, probably both).

@jwaiton jwaiton force-pushed the include-WD1-processing branch from 5dceacf to 761d187 Compare March 18, 2025 23:18
@jwaiton
Copy link
Member Author

jwaiton commented Apr 4, 2025

Test coverage now near 100%, although works on a 'whole function' basis, due to the reader, writers, and the decoder having limited compartmentalisation. Improved functionality would be nice, but currently it works as expected. I'd say its ready for a 'light' review, with the knowledge that almost every issue found will become technical debt :)

@jwaiton jwaiton requested a review from bpalmeiro April 4, 2025 13:47
@jwaiton jwaiton force-pushed the include-WD1-processing branch from a64f7d3 to c3c74fe Compare April 4, 2025 14:51
@jwaiton
Copy link
Member Author

jwaiton commented Apr 22, 2025

I've split this PR up into two separate ones (#40 & #41), to help reviews flesh out more easily.

@jwaiton jwaiton closed this Apr 22, 2025
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.

2 participants