Pair developing HDF5 #181 #250

samcunliffe · 2023-03-09T12:03:43Z

Resolves #181. Relates to #70.

samcunliffe · 2023-03-10T15:58:07Z

.github/workflows/ci.yml

+        if: ${{ contains(matrix.os, 'ubuntu') }}
+        run: |
+          sudo apt-get update
+          sudo apt-get install libhdf5-dev


Seems to work, but we may want to statically link when compiling. (Actually, I thought we were doing this...?)

samcunliffe · 2023-03-24T17:25:07Z

tdms/include/hdf5_io.h

+    // get the dataset and dataspace
+    H5::DataSet dataset = file_->openDataSet(dataset_name);
+    H5::DataSpace dataspace = dataset.getSpace();


Need to handle the case where this is a H5::Group, I think.

Co-authored-by: Mosè Giordano <giordano@users.noreply.github.com> Co-authored-by: Will Graham <32364977+willGraham01@users.noreply.github.com>

Basic class structure and some failing tests for TDD.

…se of stupid, not TDD.

With unfortunate duplication of memory into buffers. To be fixed in the future with flattened classes.

* 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

…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.

* 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.

* Rework DispersiveMultiLayer class into a struct. TDMS builds. - xyz_vector struct of 3 vector<double>s introduced - DispersiveMultiLayer is now a struct with xyz_vector members - Preserve method for checking if the medium is dispersive - Temporarily disable unit tests for DispersiveMultiLayer class - HDF5Reader::read() can now assemble a DispersiveMultiLayer - HDF5Reader has a method for reading directly into a vector<T> - H5Dimension can be passed a H5::DataSet as well as a H5::DataSpace * Update paths now we need HDF5 data as well as matlab data * TDMS builds and tests HDF5Reader::read_data_from_group (passes, no) * Fix seg-fault in read_dataset_from_group * Add DispersiveMultiLayer test * Update DispersiveMultiLayer tests to not be entangled with the MATLAB jargon * Update the CI so things actually work * Move content of benchmark_scripts readme into doc/developers * Update CI to produce unit test `.mat` and `.hdf5` data (#289) * Move content of benchmark_scripts readme into doc/developers * Update CI attempt 1 * Actually use the right syntax * Force python version to prevent MacOS using python2.7 * Rename files to better match the data they produce * Aplease windows syntax * Remove pip cache due to known windows bug: actions/setup-python#436 * Apply suggestions from code review

codecov · 2023-05-19T09:23:38Z

Codecov Report

Patch coverage: 70% and project coverage change: +24 🎉

Comparison is base (ab6cf65) 25% compared to head (e414f1c) 48%.

❗ Current head e414f1c differs from pull request most recent head 4e12af0. Consider uploading reports for the commit 4e12af0 to get more accurate results

Additional details and impacted files

@@          Coverage Diff          @@
##           main   #250     +/-   ##
=====================================
+ Coverage    25%    48%    +24%     
=====================================
  Files        65     72      +7     
  Lines      3627   7931   +4304     
=====================================
+ Hits        893   3817   +2924     
- Misses     2734   4114   +1380

Impacted Files	Coverage Δ
tdms/include/arrays.h	`26% <0%> (-6%)`	⬇️
tdms/include/cuboid.h	`0% <0%> (ø)`
tdms/include/hdf5_io/hdf5_base.h	`0% <0%> (ø)`
tdms/include/input_matrices.h	`0% <ø> (ø)`
tdms/include/interface.h	`0% <0%> (ø)`
tdms/include/output_matrices/output_matrices.h	`0% <ø> (ø)`
...s/include/simulation_manager/objects_from_infile.h	`0% <ø> (ø)`
tdms/include/hdf5_io/hdf5_reader.h	`27% <27%> (ø)`
tdms/src/input_matrices.cpp	`82% <50%> (+82%)`	⬆️
tdms/src/hdf5_io/hdf5_base.cpp	`77% <77%> (ø)`
... and 6 more

... and 51 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

willGraham01 · 2023-05-19T09:30:12Z

Cherry pick and put this into main in sensible chunks:

5503ad7 to e7bb3f0
ee9962b to 5094596. THIS SHOULD INCLUDE A NOTE ABOUT THE MAGIC -v7.3! Add a mention of MATLAB output file version 7.3 #291
Remaining commits are individual PRs that can go in one after the other.

Made it ready for review, then decided to not do that since the merge queue will eat it up.

samcunliffe · 2023-05-26T15:44:45Z

Closing as superseded by:

samcunliffe added the enhancement New feature or request label Mar 9, 2023

samcunliffe added this to the Removing MATLAB C++ header dependency milestone Mar 9, 2023

samcunliffe force-pushed the 181-hdf5-io-pairdev branch 2 times, most recently from 1864c00 to 113f9ca Compare March 9, 2023 14:01

samcunliffe commented Mar 10, 2023

View reviewed changes

samcunliffe commented Mar 24, 2023

View reviewed changes

willGraham01 and others added 19 commits May 19, 2023 10:02

Add libhdf5.

5503ad7

Co-authored-by: Mosè Giordano <giordano@users.noreply.github.com> Co-authored-by: Will Graham <32364977+willGraham01@users.noreply.github.com>

Skeleton HDF5 wrapper class and failing tests.

0303bf7

Basic class structure and some failing tests for TDD.

WIP #181. Change the interface a bit. Tests still fail but some becau…

7c310aa

…se of stupid, not TDD.

Passes first tests! #181

7d115da

Also test overwrite.

cf55e80

Adding some HDF5 tests.

ac9dbcb

Old API.

45cf98b

Can read and write a matrix.

cf8eb7d

With unfortunate duplication of memory into buffers. To be fixed in the future with flattened classes.

Perhaps we need to install HDF5 explicitly?

c440128

Planceholder for fdtdgrid read in from .mat file

43d0d9a

It compiles.

e7bb3f0

Add a smol fdtdgrid.mat file and make tests read from it

ee9962b

Try to find that file

ac12693

This should do a work

c03cdbf

Well, it now broke again. fdtdgrid y u no DataSpace

5094596

willGraham01 force-pushed the 181-hdf5-io-pairdev branch from 05ef9fc to 30899d5 Compare May 19, 2023 09:07

willGraham01 marked this pull request as ready for review May 19, 2023 09:27

willGraham01 marked this pull request as draft May 19, 2023 09:27

Add line that was lost in rebase

e414f1c

samcunliffe mentioned this pull request May 19, 2023

Add dependence on libhdf5 and basic implementation class structure. #304

Merged

willGraham01 changed the base branch from main to 181-hdf5-first-part-extracted-from-pairdev-working-branch May 19, 2023 14:03

willGraham01 changed the base branch from 181-hdf5-first-part-extracted-from-pairdev-working-branch to main May 19, 2023 14:03

Merge branch 'main' into 181-hdf5-io-pairdev

4e12af0

willGraham01 mentioned this pull request May 25, 2023

Detach XYZVector from MATLAB #315

Merged

samcunliffe mentioned this pull request May 26, 2023

DispersiveMultiLayer no longer uses MATLAB #324

Merged

samcunliffe closed this May 26, 2023

samcunliffe deleted the 181-hdf5-io-pairdev branch June 5, 2023 17:21

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Pair developing HDF5 #181 #250

Pair developing HDF5 #181 #250

samcunliffe commented Mar 9, 2023

samcunliffe Mar 10, 2023

samcunliffe Mar 24, 2023

codecov bot commented May 19, 2023 •

edited

Loading

willGraham01 commented May 19, 2023 •

edited

Loading

samcunliffe commented May 26, 2023

Pair developing HDF5 #181 #250

Pair developing HDF5 #181 #250

Conversation

samcunliffe commented Mar 9, 2023

samcunliffe Mar 10, 2023

Choose a reason for hiding this comment

samcunliffe Mar 24, 2023

Choose a reason for hiding this comment

codecov bot commented May 19, 2023 • edited Loading

Codecov Report

willGraham01 commented May 19, 2023 • edited Loading

samcunliffe commented May 26, 2023

codecov bot commented May 19, 2023 •

edited

Loading

willGraham01 commented May 19, 2023 •

edited

Loading