Infrastructure for removing MATLAB
#281
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
willGraham01
changed the title
FrequencyVectorFrequencyVectors
willGraham01
force-pushed
the
wgraham-181-less_matlab_more_hdf5
branch
from
e79a6b3
to
2782316
Compare
willGraham01
changed the title
FrequencyVectors
willGraham01
changed the title
MATLAB
41 tasks
Base automatically changed from
wgraham-181-moar-hdf5-things
to
181-hdf5-io-pairdev
May 12, 2023 12:13
samcunliffe
approved these changes
May 12, 2023
…bers. [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
…ETHOD 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
- Update HDF5Base::shape_of to return instances of this class - Add unit tests for class - Update HDF5Reader method to avoid recycled code
- simulation_manager can now instantate these members using HDF5Reader rather than still relying on the old InputMatrices object
- 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 <1willgraham@gmail.com>
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 <1willgraham@gmail.com>
Date: Fri May 5 15:13:48 2023 +0100
Fix faulty logic thrown up by test
commit b37a89f031519c6a356b50f91a12ad8867b78333
Author: willGraham01 <1willgraham@gmail.com>
Date: Fri May 5 15:01:58 2023 +0100
Allow tdms to actually compile
commit d7c982b9165e3ae4ccf3f42e0fb8c5cdde55b644
Author: willGraham01 <1willgraham@gmail.com>
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 <1willgraham@gmail.com>
Date: Fri May 5 14:12:11 2023 +0100
Change Cuboid to be a struct because a class is overkill
- setup_unit_tests.m now calls all the other data-generating scripts in the folder to save updating the ci.yaml each time.
samcunliffe
force-pushed
the
wgraham-181-less_matlab_more_hdf5
branch
from
e810276
to
f6789ab
Compare
willGraham01
added a commit
that referenced
this pull request
May 19, 2023
* 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 <1willgraham@gmail.com>
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 <1willgraham@gmail.com>
Date: Fri May 5 15:13:48 2023 +0100
Fix faulty logic thrown up by test
commit b37a89f031519c6a356b50f91a12ad8867b78333
Author: willGraham01 <1willgraham@gmail.com>
Date: Fri May 5 15:01:58 2023 +0100
Allow tdms to actually compile
commit d7c982b9165e3ae4ccf3f42e0fb8c5cdde55b644
Author: willGraham01 <1willgraham@gmail.com>
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 <1willgraham@gmail.com>
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.
samcunliffe
pushed a commit
that referenced
this pull request
May 23, 2023
* 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 <1willgraham@gmail.com>
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 <1willgraham@gmail.com>
Date: Fri May 5 15:13:48 2023 +0100
Fix faulty logic thrown up by test
commit b37a89f031519c6a356b50f91a12ad8867b78333
Author: willGraham01 <1willgraham@gmail.com>
Date: Fri May 5 15:01:58 2023 +0100
Allow tdms to actually compile
commit d7c982b9165e3ae4ccf3f42e0fb8c5cdde55b644
Author: willGraham01 <1willgraham@gmail.com>
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 <1willgraham@gmail.com>
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.
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 <1willgraham@gmail.com> 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 <1willgraham@gmail.com> Date: Fri May 5 15:13:48 2023 +0100 Fix faulty logic thrown up by test commit b37a89f031519c6a356b50f91a12ad8867b78333 Author: willGraham01 <1willgraham@gmail.com> Date: Fri May 5 15:01:58 2023 +0100 Allow tdms to actually compile commit d7c982b9165e3ae4ccf3f42e0fb8c5cdde55b644 Author: willGraham01 <1willgraham@gmail.com> 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 <1willgraham@gmail.com> 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 <samcunliffe@users.noreply.github.com> * 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 <samcunliffe@users.noreply.github.com> * Update tdms/src/input_matrices.cpp Co-authored-by: Sam Cunliffe <samcunliffe@users.noreply.github.com> * Clang format. --------- Co-authored-by: Will Graham <32364977+willGraham01@users.noreply.github.com> Co-authored-by: willGraham01 <1willgraham@gmail.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Context/Description
Building on #280, this PR:
FrequencyVectorsclass.Cuboidclass.HDF5Reader::read()andHDF5Writer::write()that can be employed going forward.There are a lot of classes to work through, but now we have a modular system which should allow us to work through each class individually.
On which note, a lot of the "classes" we have setup to handle MATLAB objects don't actually need to be classes any more by the looks of things.
C++structs, or evenpublic std::vector<>classes are sufficient for most of our purposes now thatHDF5is playing nicely.Key Changes
FrequencyVectorsis now astructbecause it being a class was overkill. Additionally, it usesstd::vectors rather than our customVectortype for it's members because we no longer need MATLABCuboidis now astructbecause a class was overkill. It could also be argued that having it's own file,shapes.hwas also overkill, but I've preserved this for now (although renamed it tocuboid.h.shapes.cppdid not survive the purge.H5Dimensionclass has been added to deal with repetition when fetching array sizes from the input files. Also allows us to have some convenient wrappers likeis_1D,max_element, etc.InputMatricesnow stores the name of the input file that is passed to it. This is an unfortunate halfway-step between complete MATLAB removal and where we are now: we cannot removeInputMatricesentirely, butsimulation_managerstill needs to be able to read in the classes that we have yet to disentangle from MATLAB. As we remove MATLAB from more classes, we will reach a point where it is sufficient to just pass the input file tosimulation_managerdirectly, andInputMatricescan be removed from the codebase as we don't need to translate to MATLAB anymore.Testing
test_FrequencyVectors.cpphas been removed since this is a) no longer a class and b) no longer uses custom methods on its members.H5Dimensions, andHDF5Reader::read()when applied toFrequencyVectorsandCuboidobjects have been added..matdata for the unit tests have been relocated (as have the paths at which they are stored) to one place rather than across a couple of directories.Unit test binaries
matlab-actions/run-command@v1to produce the binaries that the unit tests are looking for on CI rather than tracking them in the repositorycreate_structure_array.mandcreate_tdms_object_data.min thetests/unit/matlab_benchmark_scripts/directory.CI checks
CI checks might fail on the system tests until #262 is merged in (due to Cache update). However the
build-and-teststage should pass on all OSs.