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

HDF5 file structure reorganisation, and InterfaceComponent readability #280

Merged
merged 6 commits into from
May 12, 2023

Conversation

willGraham01
Copy link
Collaborator

@willGraham01 willGraham01 commented May 3, 2023

  • Allows us to directly read InterfaceComponents (objects that are saved as the fields of structs) via HDF5 as opposed to via the MATLAB headers.
  • Reorganises the HDF5 files that we're adding to the repository, since there's a lot of things going in now and there was previously ~0% organisation on our part.

Moves towards #70

Conflated two separate changes in one PR (InterfaceComponent things and a file restructure)... sorry 😅

Notes

We could now remove completely separate MATLAB headers from InterfaceComponents everywhere in the codebase, but this currently conflicts with #253 and #262 (since these need to touch the SimulationManager and associated classes with updates to the input file format).

Testing

Existing unit tests are moved around a bit so they don't just get dumped in one file.

Adds unit tests for HDF5Reader::read() when reading InterfaceComponents:

  • read(string plane, InterfaceComponent *ic) reads information from the H5 file into an existing InterfaceComponent.
  • InterfaceComponent ic = HDF5Reader.read(string plane) does the same thing as the above but creates a new InterfaceComponent instance in the process.

Documentation

  • Added some docstrings to our existing HDF5 functionality.
  • Added docstrings to interface.h because I was there anyway.

- Move uint16 to char/str functions to the unit test utils file
- Create hdf5_and_tdms_objects subdirectory of unit/ to hold unit tests on interaction between hdf5 and tdms classes
- Move the Matrix<double> test from test_hdf5_io into the new subdirectory
- Data files needed for unit tests are defined in a unit_test_utils namespace to avoid redefinition across multiple files
- Add docstrings to interface.h since these are missing and I've just had to work out what they do
- Create a matlab script that can reproduce the class_data.mat file which the hdf5 unit tests will try to create tdms objects from
- Create the barebones test_hdf5_interface file
@willGraham01 willGraham01 changed the title Wgraham 181 moar hdf5 things HDF5 file structure reorganisation, and InterfaceComponent readability May 3, 2023
@willGraham01 willGraham01 force-pushed the wgraham-181-moar-hdf5-things branch from f4397b9 to e012dab Compare May 3, 2023 14:54
- Adds scripts to reproduce this data, so in theory a new user can run a short MATLAB script to reproduce this
- Had a play with trying to get setup-matlab to run these scripts before the unit tests, but alas, no.
@samcunliffe samcunliffe merged commit 9199e46 into 181-hdf5-io-pairdev May 12, 2023
@samcunliffe samcunliffe deleted the wgraham-181-moar-hdf5-things branch May 12, 2023 12:13
willGraham01 added a commit that referenced this pull request May 19, 2023
…ility (#280)

* Reorganise hdf5 unit tests

- Move uint16 to char/str functions to the unit test utils file
- Create hdf5_and_tdms_objects subdirectory of unit/ to hold unit tests on interaction between hdf5 and tdms classes
- Move the Matrix<double> test from test_hdf5_io into the new subdirectory
- Data files needed for unit tests are defined in a unit_test_utils namespace to avoid redefinition across multiple files

* Create file to test interface and hdf5 interactions

- Add docstrings to interface.h since these are missing and I've just had to work out what they do
- Create a matlab script that can reproduce the class_data.mat file which the hdf5 unit tests will try to create tdms objects from
- Create the barebones test_hdf5_interface file

* HDF5Reader can read from .mat file and produce an InterfaceComponent

* File restructure: accounting for how many tests we are going to have with HDF5

* Prune includes

* Add .mat file for HDF5-TDMS-object unit tests to run.

- Adds scripts to reproduce this data, so in theory a new user can run a short MATLAB script to reproduce this
- Had a play with trying to get setup-matlab to run these scripts before the unit tests, but alas, no.
samcunliffe pushed a commit that referenced this pull request May 22, 2023
…ility (#280)

* Reorganise hdf5 unit tests

- Move uint16 to char/str functions to the unit test utils file
- Create hdf5_and_tdms_objects subdirectory of unit/ to hold unit tests on interaction between hdf5 and tdms classes
- Move the Matrix<double> test from test_hdf5_io into the new subdirectory
- Data files needed for unit tests are defined in a unit_test_utils namespace to avoid redefinition across multiple files

* Create file to test interface and hdf5 interactions

- Add docstrings to interface.h since these are missing and I've just had to work out what they do
- Create a matlab script that can reproduce the class_data.mat file which the hdf5 unit tests will try to create tdms objects from
- Create the barebones test_hdf5_interface file

* HDF5Reader can read from .mat file and produce an InterfaceComponent

* File restructure: accounting for how many tests we are going to have with HDF5

* Prune includes

* Add .mat file for HDF5-TDMS-object unit tests to run.

- Adds scripts to reproduce this data, so in theory a new user can run a short MATLAB script to reproduce this
- Had a play with trying to get setup-matlab to run these scripts before the unit tests, but alas, no.
samcunliffe added a commit that referenced this pull request May 23, 2023
…ility (#312)

* Handle structs (except with complex data, that's nasty) (#278)

* Trying to decipher structs

* Further towards reading a struct

* Bypass the stack-smash bug.

MATLAB saves character arrays as uint16s, and HDF5 is unable to interpret them as chars. As such, we need to do manual conversion in the test.

* Rename binary and update contents

* `HDF5` file structure reorganisation, and `InterfaceComponent` readability (#280)

* Reorganise hdf5 unit tests

- Move uint16 to char/str functions to the unit test utils file
- Create hdf5_and_tdms_objects subdirectory of unit/ to hold unit tests on interaction between hdf5 and tdms classes
- Move the Matrix<double> test from test_hdf5_io into the new subdirectory
- Data files needed for unit tests are defined in a unit_test_utils namespace to avoid redefinition across multiple files

* Create file to test interface and hdf5 interactions

- Add docstrings to interface.h since these are missing and I've just had to work out what they do
- Create a matlab script that can reproduce the class_data.mat file which the hdf5 unit tests will try to create tdms objects from
- Create the barebones test_hdf5_interface file

* HDF5Reader can read from .mat file and produce an InterfaceComponent

* File restructure: accounting for how many tests we are going to have with HDF5

* Prune includes

* Add .mat file for HDF5-TDMS-object unit tests to run.

- Adds scripts to reproduce this data, so in theory a new user can run a short MATLAB script to reproduce this
- Had a play with trying to get setup-matlab to run these scripts before the unit tests, but alas, no.

* Some tidyup from the comments of PR #312.

Co-authored-by: Will Graham <[email protected]>

---------

Co-authored-by: Will Graham <[email protected]>
Co-authored-by: willGraham01 <[email protected]>
samcunliffe added a commit that referenced this pull request May 26, 2023
* Handle structs (except with complex data, that's nasty) (#278)

* Trying to decipher structs

* Further towards reading a struct

* Bypass the stack-smash bug.

MATLAB saves character arrays as uint16s, and HDF5 is unable to interpret them as chars. As such, we need to do manual conversion in the test.

* Rename binary and update contents

* `HDF5` file structure reorganisation, and `InterfaceComponent` readability (#280)

* Reorganise hdf5 unit tests

- Move uint16 to char/str functions to the unit test utils file
- Create hdf5_and_tdms_objects subdirectory of unit/ to hold unit tests on interaction between hdf5 and tdms classes
- Move the Matrix<double> test from test_hdf5_io into the new subdirectory
- Data files needed for unit tests are defined in a unit_test_utils namespace to avoid redefinition across multiple files

* Create file to test interface and hdf5 interactions

- Add docstrings to interface.h since these are missing and I've just had to work out what they do
- Create a matlab script that can reproduce the class_data.mat file which the hdf5 unit tests will try to create tdms objects from
- Create the barebones test_hdf5_interface file

* HDF5Reader can read from .mat file and produce an InterfaceComponent

* File restructure: accounting for how many tests we are going to have with HDF5

* Prune includes

* Add .mat file for HDF5-TDMS-object unit tests to run.

- Adds scripts to reproduce this data, so in theory a new user can run a short MATLAB script to reproduce this
- Had a play with trying to get setup-matlab to run these scripts before the unit tests, but alas, no.

* Infrastructure for removing `MATLAB` (#281)

* Skeleton for new method

* FrequencyVectors use std::vectors instead of our custom object as members. [DOESN'T COMPILE YET]

- FrequencyVectors is now just a struct that uses std::vector datatypes
- SimualationManager is broken, along with most parts of the codebase that use FrequencyVectors
- Pending update to allow code to COMPILE

* Allow TDMS to actually be compiled again. [TESTS STILL FAIL, READER METHOD NOT READY YET]

- Move .mat data generation scripts for unit tests into benchmarking folder
- Adjust paths to unit test data accordingly
- InputMatrices stores the filename so that we can minimise disruption as we transition away from MATLAB

* IT WORKS

* Create the abstract H5Dimensions class to ease reading objects.

- Update HDF5Base::shape_of to return instances of this class
- Add unit tests for class
- Update HDF5Reader method to avoid recycled code

* Remove MATLAB pointers to InterfaceComponents in initialisation.

- simulation_manager can now instantate these members using HDF5Reader rather than still relying on the old InputMatrices object

* Add docstrings to files touched

* Produce unit test binaries on CI rather than track in repo

* - Detaches MATLAB from Cuboid class.
- Turns Cuboid class into a struct because MATLAB is now gone.
- Adds failure-case checks to HDF5Reader::read() functions.

Squashed commit of the following:

commit ce8d15e311e9dbb93bd7d7fbcd6c9cffe69657e3
Author: willGraham01 <[email protected]>
Date:   Fri May 5 15:19:51 2023 +0100

    Rename shapes.h to cuboid.h because that's all it contains

commit fe83f568c3da7491d450693574975df90a3f15e1
Author: willGraham01 <[email protected]>
Date:   Fri May 5 15:13:48 2023 +0100

    Fix faulty logic thrown up by test

commit b37a89f031519c6a356b50f91a12ad8867b78333
Author: willGraham01 <[email protected]>
Date:   Fri May 5 15:01:58 2023 +0100

    Allow tdms to actually compile

commit d7c982b9165e3ae4ccf3f42e0fb8c5cdde55b644
Author: willGraham01 <[email protected]>
Date:   Fri May 5 14:31:43 2023 +0100

    Write unit test and read method for Cuboid. Add failure test for FrequencyVectors

    - Add new .m script to produce an input file with bad object data
    - Update unit_test_utils with this new filepath
    - Write HDF5Reader(Cuboid *cube) method and unit test
    - Add failure-test check for FrequencyVectors using bad data file

commit cf61970d2052ab02ae23822f0edd28e89044734f
Author: willGraham01 <[email protected]>
Date:   Fri May 5 14:12:11 2023 +0100

    Change Cuboid to be a struct because a class is overkill

* Use one master script for setting up .mat files for unit tests.

- setup_unit_tests.m now calls all the other data-generating scripts in the folder to save updating the ci.yaml each time.

* Apply suggestions from code review

Co-authored-by: Sam Cunliffe <[email protected]>

* Move cuboid class into array/ directory, which will be populated as we deatch other classes from MATLAB

* Update tdms/src/input_matrices.cpp

Co-authored-by: Sam Cunliffe <[email protected]>

* Update tdms/src/input_matrices.cpp

Co-authored-by: Sam Cunliffe <[email protected]>

* Clang format.

---------

Co-authored-by: Will Graham <[email protected]>
Co-authored-by: willGraham01 <[email protected]>
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