Skip to content

Refactor [2/N] #88

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

Merged
merged 24 commits into from
Oct 10, 2022
Merged

Refactor [2/N] #88

merged 24 commits into from
Oct 10, 2022

Conversation

t-young31
Copy link
Member

Continues #63

I'd strongly recommend we add some more comprehensive system tests before merging this.


Guidelines for reviewers

These commits have been reviewed as separate PRs on my fork previously. If this PR warrants further review I'd strongly recommend reviewing the individual commits, with the exception of the one that just clang-formats the entire iterator.cpp file.


Extensions

  • Convert the cerr 'prints' to spdlog logging calls
  • Cary on the refactor past the start of the main loop
  • Change the non-matlab arrays (defined in arrays.h) to something more robust
  • Use the defined .clang-format file to reformat the entire code base (ideally as a precommit hook)
  • Add many more unit tests (this refactor largely focused on extracting classes and structs that have matlab-dependent constructors, thus didn't warrant testing, if they're going to be rewritten)

Dependencies

t-young31 and others added 15 commits September 9, 2022 17:04
Remove unused pointers


Don't throw with unused


Add pkg executable

Refactor numeric.cpp


Refactor numerical_derivative



Start iterator refactor

Add timers

Add catch2 fetch message

Don't narrow int


Doc + indentation + type decl


Delete unused pragmas on CI


Add simulation params struct remove unused globals


Extract angular norms


Add H field test


Fix time order

Use relative difference


Use built in pi constant


Use langle rangle


auto types


Include math defines


Delete unused code


Fix typo in tdms/include/globals.h

Revert pi import
* Apply useCD as a compile time flag

* Extract grid labels

* Use E/H classes for split field pointers

* Add ft to E/H attributes

* Separate split and regular fields

* Use E/H classes in functions
* Add destructors and allocation to split field

* Delete unused function declaration

* Use split fields in extractPhasors

Co-authored-by: David Stansby <[email protected]>
* Convert tabs to spaces

* clang format
* Add test for allocation and zeroing

* Loops over components

* Associate bounds with E

* Make normalise volume a method

* Fix typo

* Change int to bools

* Extract ex_tdf save

* Fix dividing by zero
* Extract init grid tensors

* Remove unused file

* Use a zero function on exported fields

* Add index bounds to fields

* Add docstrings
* Use a shared phasor extract function

* Use index operator for indexing

* Delete unused code

* Delete unused yaml if

* Move timer doc to header

* Delete more unused code
* Extract C and D materials
* Extract freespace and alpha beta gamma

* Extract delta

* Extract interface components

* Remove unused array

* Rename xyz_arrays

* Extract source tensors
* Extract grid labels

* Extract omega_an

* Extract dt and hwhm

* Extract pml and Nt

* Extract start tind
* Extract phasor extraction

* Use ++ to increment argument counter

* Use unsigned ints for iteration

* Add enums and set methods

* Extract rho

* Extract ml

* Extract cuboid

* Reorder dimension enum

Co-authored-by: Sam Cunliffe <[email protected]>
* Use templated array casts and frees

* Extract structure

* Extract f_ex_vec

* Extract f_vec

* Renaming

* Template vectors and matrices

* Extract pupil

* Extract D_tilde

* Apply doc improvements from code review [skip actions]

Co-authored-by: Sam Cunliffe <[email protected]>
* Extract simple parameters

* Extract interpolation method

* Change extractPhasorsSurface signature

* Extract exi eyi

* Use string_in

* Extract phasorinc

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

* Extract campssample
* Extract Ex_t Ey_t

* Extract Np Npe

* Extract cx_vec

* Extract eh_vec

* Extract fftw plan

* Use bool not int flags

* Use variable for indexing
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.

Just reminding myself of what's going on here (and double-checking for anything I might've missed).

→ I've already reviewed all of the changes in little chunks: so approving here too.

@samcunliffe
Copy link
Member

Conflicts.

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.

3 participants