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

More unit tests for MATLAB replacer classes #171

Merged
merged 55 commits into from
Nov 17, 2022

Conversation

willGraham01
Copy link
Collaborator

@willGraham01 willGraham01 commented Nov 9, 2022

Vertices class | Closes #170

  • initialise() successfully assigns memory and decrements all stored values by 1, to be consistent with C++, rather than MATLAB indexing.

FieldComponentsVector class | Closes #172

  • Instances can be initialised from structs that contain the appropriate fields, and have the expected sizes.
  • Can handle both vertical and horizontal arrays being passed in.

ComplexAmplitudeSample class | Closes #173

  • Construction via the empty struct does not cause an error, but exits construction appropriately.
  • Otherwise, a struct with two fields provided will correctly initialise the attributes components and vertices.

GratingStructure class | Closes #174

  • Fixes a double-deallocation bug when assigning from MATLAB arrays
  • Checks appropriate construction/destruction/assignment methods

Pupil class | Closes #175

  • Checks initialise() method performs correctly, given the MATLAB input
  • Fixes a double-deallocation bug

EHVec class | Closes #176

  • Can correctly assign fftw_complex to the entries of the object
  • fftw_free used in destructor

FrequencyExtractVector class | Closes #177

  • Constructor can handle both vertical and horizontal arrays
  • max() function correctly identifies maximal element
  • Data can be read in from MATLAB datastructs

CCollection class | Closes #178

  • Error thrown when struct passed is of the wrong shape/size
  • Correctly extracts components when fields are labelled correctly
  • is_disp_ml and is_multilayer are "correctly" identified, although has flagged a logical flaw in one of these

DCollection class | Closes #179

  • Similar functionality to CCollection, minus the possibility of being provided with 9 fields over 6.

CMaterial class | Closes #184

  • Construction with incorrect fields throws an error
  • Updates common method in matlabio.cpp to avoid seg-faults if the input has an incorrect fieldname

DMaterial class | Closes #185

  • Literally identical to CMaterial barring a 6 instead of a 9.

@willGraham01 willGraham01 linked an issue Nov 14, 2022 that may be closed by this pull request
@samcunliffe samcunliffe added the testing Adding or requesting more test coverage label Nov 14, 2022
@willGraham01 willGraham01 marked this pull request as ready for review November 14, 2022 11:09
Base automatically changed from wgraham-more_array_tests to main November 16, 2022 16:17
Copy link
Member

@samcunliffe samcunliffe left a comment

Choose a reason for hiding this comment

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

LGTM! Despite my picky comments about using full words for variables these are all nice tests.

Just to be clear: both me and Google prefer full names wherever possible...

Do not worry about saving horizontal space as it is far more important to make your code immediately understandable by a new reader.

Perhaps the only exception for us is if it's actually the name of a maths symbol from Peter's thesis/convention e.g. $C$, $D$. (Although even then we should probably try to understand the object and name it.)

.github/workflows/linux_tests.yml Outdated Show resolved Hide resolved
.github/workflows/macos_tests.yml Outdated Show resolved Hide resolved
tdms/src/arrays.cpp Outdated Show resolved Hide resolved
tdms/src/arrays.cpp Outdated Show resolved Hide resolved
tdms/tests/unit/array_tests/test_Matrix.cpp Outdated Show resolved Hide resolved
tdms/tests/unit/array_tests/test_Matrix.cpp Outdated Show resolved Hide resolved
tdms/tests/unit/array_tests/test_Tensor3D.cpp Outdated Show resolved Hide resolved
tdms/tests/unit/array_tests/test_Vector.cpp Outdated Show resolved Hide resolved
tdms/tests/unit/array_tests/test_Vector.cpp Outdated Show resolved Hide resolved
willGraham01 and others added 3 commits November 17, 2022 10:14
Will need to go through and check for consistency

Co-authored-by: Sam Cunliffe <[email protected]>
@willGraham01 willGraham01 merged commit abf6632 into main Nov 17, 2022
@willGraham01 willGraham01 deleted the wgraham-even_more_array_tests branch November 17, 2022 11:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment