Update `iteratefdtd_matrix`: `dz/2` offset and system test input/output update #253

willGraham01 · 2023-03-10T11:23:25Z

Closes #240, #259, #260 | Moves towards #208 and #217
Also closes #169

Sorry about the large diff, but a lot of it is just config file updates and the like. The features this touches are all interlinked, so we can't solve one without upsetting the others.

Features

Bugfixes the optimise_J_loop bugs
- Checks Ksource is non-empty before trying to access data
- Source can be accessed via the [] operator, returning the complex value that Source.real[k][j][i] + i * Source.imag[k][j][i] represents
- Source also has a value_or_zero_if_empty method, which accounts for the new functionality where the Source arrays are permitted to be empty and implicitly zero.
- nz_ksource (the array used in optimising the loop variables) has undergone a cleanup in how its elements are assigned
Adds option to "adjust" existing .mat inputs during the regeneration process
- .mat inputs that contain identical data but different "flag" variables (that replace the CLI flags) can be produced without re-running the run_bscan script. Instead, Python can use matload and matsave to toggle these values
- config_XX.yaml files adjusted to the new "adjust" syntax.
- tdms/tests/system/README.md updated accordingly
Fixes the previously undiscovered Bugs and output changes caught in update to iteratefdtd_matrix.m #260.
Updates the MATLAB tests to account for the new iteratefdtd_matrix behaviour as per arc_19
- arc_19 provided in Update to iteratefdtd_matrix #208 details the compatible inputs to iteratefdtd_matrix. The various cases are now checked for expected errors in test_iteratefdtd_matrix_source_combinations.m.
- Note that arc_19 has no system test runs, IE no calls to tdms for reference inputs/outputs.
General tidy-up of the iteratefdtd_matrix function.
- Removes eval statements and replaces them with explicit function-handle casts.
- Improves logic control and refactors some functionality to avoid repeated code.
- Implements MATLAB-linter recommended code improvements

Updating the reference outputs

The reference inputs and outputs will need to be replaced on Zenodo pending this update, since the input .mat files have changed. There's a brief log of the differences induced in the output files here:
input_file_changes.log
The major changes are largely reconciling hardcoded values of dz, which means the numerical data in the .mat input files is now different, and the reference outputs correspondingly updated. Adding compactsource to all the input files, and some illumination setup options, are the other changes.

When this is approved, the GitHub cache will need to be cleared, the new data files uploaded to Zenodo, and the test_system.py and test_regen.py links updated to point to the new data. This should force GitHub to re-download the new input data and references.

- optimise_J_loop_variables gains additional checks against accessing nullptrs - test_regen.py flushes correctly (so we know if regen has failed prior to the 1st test starting) - objects_from_infile destructor only deallocates if there is memory to deallocate - Source methods changed to const

- Config files for 01-13, example_fdtd updated - test_regen temporarily ignores arc_17,18 - matfile_option_edit.py is created, allowing us to edit single-value variables in existing .mat files - iteratefdtd_matrix updated to at least save usecd variable, even though it is still not used by tdms

Updates have been made to arc_01 to be in-line with the dz/2 bug fix: - efield functions no longer multiply field by 2 and offset by dz/2 - new iteratefdtd_matrix is added to overwrite the old one (although we should go back and reconcile this with the MATLAB tests we currently employ) HOWEVER: - It looks like some of the dz/2 hardcoded values did not match the delta.z values that come in from the input file. arc_01 can demo this: delta.z = lambda/4, but it was using dz = lambda/6 in e_field_gauss! As such, the test now "fails" because the input file generated is different, ergo the output is different from the reference. You can even notice that, if you change iteratefdtd_matrix to have delta.z/2*4/6, the arc_01 tests will all pass again!!

…efdtd_matrix

- InvalidIlluminationSourceExiEyiErrors test case is now covered by the source-input combinations test cases - IlluminationAndEfield is no longer needed - this case is now permitted as per Peter's request

- Explicit cast of efname and hfname to strings to avoid warnings - Pull out some common functionality - Refactors and adds docstrings to MATLAB functions Squashed commit of the following: commit 26308703039ecfe7e4ea18d022278be0783ad66e Author: willGraham01 <1willgraham@gmail.com> Date: Mon Apr 3 10:44:09 2023 +0100 Complete iteratefdtd_matrix refactor commit 96580be13e8a3712a6ed01d58808fddaf06db523 Author: willGraham01 <1willgraham@gmail.com> Date: Fri Mar 31 16:16:05 2023 +0100 refactor and tidy iteratefdtd_matrix... but something has broken commit 78134e7a630bfb1a53be39176da62f21b68d73ae Author: willGraham01 <1willgraham@gmail.com> Date: Fri Mar 31 16:15:46 2023 +0100 refactor and tidy iteratefdtd_matrix... but something has broken commit d3cad0cd7ff9710ba63f1b1749a9856398def8de Author: willGraham01 <1willgraham@gmail.com> Date: Fri Mar 31 15:09:56 2023 +0100 Update input file syntax to avoid MATLAB warnings commit d3e1b8f9bf9a37d34381385f5228386a84a48e57 Author: willGraham01 <1willgraham@gmail.com> Date: Fri Mar 31 13:59:02 2023 +0100 Give .m function and file same name commit fac50f86013978b25b17152c61bcaad141043e30 Author: willGraham01 <1willgraham@gmail.com> Date: Fri Mar 31 13:55:49 2023 +0100 Update docstrings and function syntax in MATLAB unit tests commit 25f0a497cacfdeb301a7be1c454636b3ba4a4e90 Author: willGraham01 <1willgraham@gmail.com> Date: Fri Mar 31 12:11:28 2023 +0100 Remove copy of pstd_input_file_2D and redirect tests appropriately commit 22bd7ea5d9ba0f11d3a8209e26458bac4020c1b5 Author: willGraham01 <1willgraham@gmail.com> Date: Fri Mar 31 12:10:11 2023 +0100 DRY test_iteratefdtd_matrix.m

- edit_mat_file function replaces wrapping class

codecov · 2023-05-04T15:28:49Z

Codecov Report

Patch coverage: 43.75% and project coverage change: +0.06 🎉

Comparison is base (d5a14c0) 48.09% compared to head (c6b4e3e) 48.15%.

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #253      +/-   ##
==========================================
+ Coverage   48.09%   48.15%   +0.06%     
==========================================
  Files          63       63              
  Lines        7804     7808       +4     
==========================================
+ Hits         3753     3760       +7     
+ Misses       4051     4048       -3

Impacted Files	Coverage Δ
tdms/include/source.h	`0.00% <0.00%> (ø)`
...dms/src/simulation_manager/objects_from_infile.cpp	`66.09% <0.00%> (ø)`
...s/src/simulation_manager/source_updates_pulsed.cpp	`71.87% <50.00%> (-0.91%)`	⬇️
tdms/src/simulation_manager/loop_variables.cpp	`80.41% <100.00%> (+1.10%)`	⬆️

... and 1 file 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-04T15:30:35Z

Codecov Report

Patch coverage has no change and project coverage change: -23.96 warning

#279 continues...

Also just to confirm: tests pass locally with the Zenodo data updated:

samcunliffe

Some of my suggestions are potentially a bit fiddly and I might have broken your words/code 😵‍💫 .

doc/developers.md

tdms/include/source.h

tdms/tests/matlab/README.md

tdms/tests/system/data/input_generation/matfile_option_edit.py

samcunliffe · 2023-05-04T16:32:30Z

tdms/tests/system/test_regen.py

+@pytest.mark.filterwarnings(
+    "ignore::DeprecationWarning"
+)  # hdf5storage.savemat throws a comparison warning from within numpy


Ah, right. The (much much) lesser of the evils.

Yeah :/ Although I can make a ticket to use h5py rather than hdf5storage to do the "adjusting".

Now we know that only -v7.3 .mat files are HDF5 (and we're enforcing saving in this convention), we can in theory have the "adjust" method read/write via h5py assuming there's no funny business with MATLAB -> Python datatypes.

samcunliffe · 2023-05-04T16:36:52Z

Codecov Report

Patch coverage has no change and project coverage change: -23.96 warning

#279 continues...

Super weird though: is that sometimes the failure causes the @CodeCovBot's check to fail permanently, and sometimes it's smart enough to notice that the coverage has updated to be more consistent and also update the check to a pass.

All of this would be a moot point if I just figured out #279.

Co-authored-by: Sam Cunliffe <samcunliffe@users.noreply.github.com>

willGraham01 mentioned this pull request Mar 10, 2023

#217 changes #246

Closed

samcunliffe mentioned this pull request Mar 10, 2023

Update to iteratefdtd_matrix #208

Closed

willGraham01 force-pushed the wgraham-217-update_ifdtdmatrix branch from 55236ab to bfa1657 Compare March 29, 2023 13:19

willGraham01 changed the title ~~Update iteratefdtd_matrix: CIL options moved to input file~~ Update iteratefdtd_matrix: dz/2 offset and system test input/output update Apr 3, 2023

willGraham01 self-assigned this Apr 3, 2023

willGraham01 marked this pull request as ready for review April 3, 2023 10:30

willGraham01 mentioned this pull request Apr 3, 2023

Update to iteratefdtd_matrix: Command line options moved to input file #217

Closed

13 tasks

willGraham01 mentioned this pull request Apr 27, 2023

Track generation for arc_02 and arc_03 #241

Merged

willGraham01 force-pushed the wgraham-add_arc_17_18 branch from 422efc1 to e5a23ae Compare May 3, 2023 09:29

willGraham01 mentioned this pull request May 3, 2023

HDF5 file structure reorganisation, and InterfaceComponent readability #280

Merged

Base automatically changed from wgraham-add_arc_17_18 to main May 4, 2023 13:07

willGraham01 added 15 commits May 4, 2023 14:10

Add usecd and intmethod flags to the input files

726c0b0

Up to dz/2 things, arc_01-09 are now working

46f9b54

Fix bug in main branch merge

b6b234c

Updates for arc_10 (assuming reference outputs now need to change)

cb80ac4

arc_12 updates

fd924ee

Update arc_13 and example_fdtd input files

281b38a

Configs updated to allow system tests to find files

0c1e7b2

Update matlab tests w/ valid combinations of Source inputs for iterat…

aaa845a

…efdtd_matrix

Refactor MATLAB unit testing folder. TODO: Recover Tom's unit tests

190d485

MATLAB tests updated and passing locally

5ff9cfd

- InvalidIlluminationSourceExiEyiErrors test case is now covered by the source-input combinations test cases - IlluminationAndEfield is no longer needed - this case is now permitted as per Peter's request

Remove illsetup extra file creation, it is no longer necessary

162c538

Rework un-neccessary class into function

2a9b38f

- edit_mat_file function replaces wrapping class

willGraham01 force-pushed the wgraham-217-update_ifdtdmatrix branch from 3c22e76 to 2a9b38f Compare May 4, 2023 15:14

Update dev docs with MATLAB test edits

9d04a91

willGraham01 requested a review from samcunliffe May 4, 2023 15:29

samcunliffe requested changes May 4, 2023

View reviewed changes

willGraham01 and others added 2 commits May 5, 2023 09:22

Apply suggestions from code review

2d71c53

Co-authored-by: Sam Cunliffe <samcunliffe@users.noreply.github.com>

Apply Sam's code review comments without breaking the PR

c6b4e3e

willGraham01 requested a review from samcunliffe May 5, 2023 08:36

samcunliffe approved these changes May 5, 2023

View reviewed changes

samcunliffe merged commit b510b06 into main May 5, 2023
11 of 12 checks passed

samcunliffe deleted the wgraham-217-update_ifdtdmatrix branch May 5, 2023 09:30

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update `iteratefdtd_matrix`: `dz/2` offset and system test input/output update #253

Update `iteratefdtd_matrix`: `dz/2` offset and system test input/output update #253

willGraham01 commented Mar 10, 2023 •

edited

codecov bot commented May 4, 2023 •

edited

willGraham01 commented May 4, 2023 •

edited

Codecov Report

samcunliffe left a comment •

edited

samcunliffe May 4, 2023

willGraham01 May 5, 2023

samcunliffe commented May 4, 2023 •

edited

Codecov Report

Update iteratefdtd_matrix: dz/2 offset and system test input/output update #253

Update iteratefdtd_matrix: dz/2 offset and system test input/output update #253

Conversation

willGraham01 commented Mar 10, 2023 • edited

Features

Updating the reference outputs

codecov bot commented May 4, 2023 • edited

Codecov Report

willGraham01 commented May 4, 2023 • edited

Codecov Report

samcunliffe left a comment • edited

Choose a reason for hiding this comment

samcunliffe May 4, 2023

Choose a reason for hiding this comment

willGraham01 May 5, 2023

Choose a reason for hiding this comment

samcunliffe commented May 4, 2023 • edited

Codecov Report

Update `iteratefdtd_matrix`: `dz/2` offset and system test input/output update #253

Update `iteratefdtd_matrix`: `dz/2` offset and system test input/output update #253

willGraham01 commented Mar 10, 2023 •

edited

codecov bot commented May 4, 2023 •

edited

willGraham01 commented May 4, 2023 •

edited

samcunliffe left a comment •

edited

samcunliffe commented May 4, 2023 •

edited