-
Notifications
You must be signed in to change notification settings - Fork 4
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
Final part of #230: clarity + the flow of information #265
Merged
Merged
Changes from all commits
Commits
Show all changes
4 commits
Select commit
Hold shift + click to select a range
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,35 @@ | ||
--- | ||
name: Bug report | ||
about: Create a report to help us improve | ||
title: '' | ||
labels: 'bug' | ||
assignees: '' | ||
|
||
--- | ||
|
||
**Describe the bug** | ||
Your best short description of what the bug is. [E.g. the z-component of the H field is not normalized correctly.] | ||
|
||
**To Reproduce** | ||
Steps to reproduce the behavior, and any input files (you can either attach them by dragging and dropping here into the issue, or upload them somewhere public). | ||
|
||
For example: | ||
1. Run this first '....' | ||
2. Then run this `tdms what_file.mat` (`what_file.mat` uploaded to this issue). | ||
3. Plot the resulting... | ||
4. See error by checking... | ||
|
||
**Expected behavior** | ||
What you expected to happen (if it's not already obvious from the context). | ||
|
||
**Plots** | ||
If applicable, add output plots to help explain your problem. | ||
|
||
**Environment (please complete the following information):** | ||
- OS: [e.g. MacOS 12.0, Windows 11, Ubuntu 23.04 LTS] | ||
- Compiler: [e.g. clang 14.0] (the full output of `g++ --version` or `clang --version` is helpful) | ||
- CMake version: (output of `cmake --version`) | ||
- MATLAB version (if relevant): [e.g. 2022a, 2022b, ...] | ||
|
||
**Additional context** | ||
Add any other context about the problem here. |
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
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,26 @@ | ||
--- | ||
name: Feature request | ||
about: Suggest an idea for TDMS | ||
title: '' | ||
labels: 'enhancement' | ||
assignees: '' | ||
|
||
--- | ||
|
||
**Is this feature related to something we're already working on?** | ||
[Delete this part if appropriate] Please take a look at our [currently open issues](https://github.com/UCL/TDMS/issues/) and see if your request is already covered, or we're already working on similar features. | ||
|
||
* Similar to # (issue number) | ||
* Relates to # (issue number) | ||
|
||
**Is your feature request related to a problem? Please describe.** | ||
A clear and concise description of what the problem is. E.g. I want to use this for ... | ||
|
||
**Describe the solution you'd like** | ||
A clear and concise description of what you want to happen. | ||
|
||
**Describe alternatives you've considered** | ||
A clear and concise description of any alternative solutions or features you've considered. | ||
|
||
**Additional context** | ||
Add any other context about the feature request here. |
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
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,22 @@ | ||
# Context/Description [delete as appropriate] | ||
|
||
Brief summary of the change and which issue is fixed. List any dependencies that are required for this change. | ||
|
||
* Fixes # (issue) | ||
* Depends on # (issue or PR) | ||
|
||
## More details | ||
|
||
[Delete if not needed] More details about the fix or feature to help us understand. | ||
|
||
## Testing | ||
|
||
Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce. | ||
|
||
- [ ] Tested with ... | ||
- [ ] Added a unit test for ... | ||
|
||
## Documentation [delete if not needed] | ||
|
||
* Added documentation inline in the code. | ||
* [Optional] added to developers documentation with... |
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
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
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
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,36 +1,31 @@ | ||
# Contribution Guidelines | ||
# Contribution guidelines | ||
|
||
### New features/Issues | ||
## Bugs | ||
|
||
To suggest a new feature or report an issue please use the | ||
[issues](https://github.com/UCL/TDMS/issues) board providing details of either | ||
the feature or the bug. | ||
🐛 If you've spotted a bug or want to request a feature please [send an issue through github](https://github.com/UCL/TDMS/issues/new). | ||
Please choose the "bug" or "feature" template (as appropriate) and fill it out with much information as you can. | ||
We really appreciate bug reports and feature requests. | ||
Thanks! | ||
|
||
For bugs, please provide detailed steps to reproduce, your operating system, and what you expected to happen. | ||
## Code contribution | ||
|
||
🚀 If you want to contribute code, or you have reported a bug and think you have a fix, then please: | ||
|
||
To contribute to the code base please | ||
1. [fork](https://docs.github.com/en/get-started/quickstart/fork-a-repo) | ||
this repository, | ||
2. commit and push the changes to your fork | ||
3. and submit a [pull request](https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/proposing-changes-to-your-work-with-pull-requests/creating-a-pull-request) (PR) against the _main_ branch, being mindful of the guidelines below. | ||
2. make your changes and check they compile, | ||
3. commit and push the changes to your fork | ||
+ (optional but appreciated) [setup and run pre-commit](https://github-pages.ucl.ac.uk/TDMS/md_doc_developers.html#pre-commit) which automates our code tidyup, | ||
4. and submit a [pull request](https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/proposing-changes-to-your-work-with-pull-requests/creating-a-pull-request) (PR) against the `main` branch. | ||
|
||
### Pull requests | ||
|
||
PRs should follow the following guidelines | ||
|
||
- Contain a **context** or **description** of the change | ||
- Include any useful hints for reviewers | ||
- Be focused on a single feature or fix, i.e. a logically consistent set of changes | ||
- Not be excessively large, ideally touching 100s not 1000s of lines | ||
- Add additional tests to ensure the correct functionality | ||
- Should pass all system and unit tests run on GitHub Actions CI | ||
- Follow the code style and have documentation as appropriate | ||
|
||
### Tests | ||
|
||
Tests are located in [`tdms/tests`](./tdms/tests) and include both unit and system tests. These | ||
must pass locally and on CI for a change to be merged. | ||
When you're submitting a PR please give as much context as possible and link to any issues of relevance. | ||
You can use our PR template as a starting point, but here are some guidelines we try to follow: | ||
|
||
### Developer documentation | ||
- Focus on a single feature or bug fix. One set of logically consistent changes per PR. | ||
- Ideally a PR should touch 100s, not 1000s, of lines. | ||
samcunliffe marked this conversation as resolved.
Show resolved
Hide resolved
|
||
- [Add test(s)](https://github-pages.ucl.ac.uk/TDMS/md_doc_developers.html#testing) to ensure the correct functionality or which reproduce the bug and demonstrate the fix. | ||
- Document your code changes, [we use doxygen for C++](https://github-pages.ucl.ac.uk/TDMS/md_doc_developers.html#code-style-and-doxygen). | ||
|
||
We have some more detailed [developer documentation](https://github-pages.ucl.ac.uk/TDMS) (C++ class structure, code style, etc) on our `gh-pages` site. | ||
You can find more information in our [developer and API documentation](https://github-pages.ucl.ac.uk/TDMS/md_doc_developers.html). |
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
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -13,110 +13,81 @@ would be duplicated) everything else in README.md is also the project homepage. | |
|
||
# Time-Domain Maxwell Solver | ||
|
||
TDMS, the Time Domain Maxwell Solver, is a hybrid C++ and MATLAB tool for solving | ||
Maxwell's equations to simulate light propagation through a medium. See the | ||
[pdf documentation](https://github.com/UCL/TDMS/blob/gh-doc/masterdoc.pdf) for | ||
further details. | ||
TDMS, the Time Domain Maxwell Solver, is a hybrid C++ and MATLAB tool for simulating light propagation through a medium by solving Maxwell's equations. | ||
For further details about the method, please refer to the [PDF documentation](https://github.com/UCL/TDMS/blob/gh-doc/masterdoc.pdf). | ||
|
||
![The normed z-component of the H field incident on a cylinder](doc/assets/HzNormBanner.png) | ||
|
||
## Getting started | ||
|
||
## Compilation | ||
|
||
TDMS needs to be built against [FFTW](https://www.fftw.org/) and | ||
[MATLAB](https://www.mathworks.com/products/matlab.html), thus both need to be | ||
downloaded and installed prior to compiling TDMS. Install with | ||
To use TDMS, it needs to be built against [FFTW](https://www.fftw.org/) and [MATLAB](https://www.mathworks.com/products/matlab.html), which must be downloaded and installed first. | ||
To install, follow these steps: | ||
|
||
```bash | ||
$ cd tdms | ||
$ git clone [email protected]:UCL/TDMS.git | ||
$ cd TDMS | ||
$ git checkout v1.0.0 # the stable version | ||
$ mkdir build; cd build | ||
$ cmake .. \ | ||
$ cmake ../tdms \ | ||
# -DMatlab_ROOT_DIR=/usr/local/MATLAB/R2019b/ \ | ||
# -DFFTW_ROOT=/usr/local/fftw3/ \ | ||
# -DCMAKE_INSTALL_PREFIX=$HOME/.local/ \ | ||
# -DBUILD_TESTING=ON | ||
# -DCMAKE_INSTALL_PREFIX=$HOME/.local/ | ||
$ make install | ||
``` | ||
where lines need to be commented in and the paths modified if cmake cannot | ||
(1) find MATLAB, (2) find FFTW or (3) install to the default install prefix. | ||
|
||
If CMake cannot find MATLAB, FFTW, or install to the default installat prefix, uncomment the relevant line(s) and modify the path(s) accordingly. | ||
|
||
<details> | ||
<summary>Mac specific instructions</summary> | ||
<summary>Mac-specific instructions</summary> | ||
|
||
To compile on a Mac an x86 compiler with libraries for OpenMP are required, | ||
which can be installed using [brew](https://brew.sh/) with `brew install llvm` | ||
then (optionally) set the following cmake arguments | ||
To compile TDMS on a Mac, you will need an x86 compiler with libraries for OpenMP. | ||
You can install these using [Homebrew](https://brew.sh) with the command: | ||
|
||
```{sh} | ||
brew install llvm | ||
``` | ||
|
||
After installing with Homebrew, you may need to set the following CMake arguments: | ||
|
||
```{sh} | ||
-DCMAKE_CXX_COMPILER=/Users/username/.local/homebrew/opt/llvm/bin/clang++ | ||
-DOMP_ROOT=/Users/username/.local/homebrew/opt/llvm/ | ||
-DCXX_ROOT=/Users/username/.local/homebrew/opt/llvm | ||
``` | ||
|
||
On an ARM Mac install the x86 version of brew with | ||
```bash | ||
On an ARM Mac, you will need to install the x86 version of Homebrew. | ||
To do so, use the following commands: | ||
|
||
```{sh} | ||
arch -x86_64 zsh | ||
arch -x86_64 /bin/bash -c "$(curl -fsSL https://raw.githubusercontent.com/Homebrew/install/HEAD/install.sh)" | ||
arch -x86_64 /usr/local/bin/brew install llvm | ||
``` | ||
</details> | ||
|
||
|
||
## Usage | ||
|
||
Once the executable has been compiled and installed, `tdms` should be in the `PATH`. | ||
Check that installation worked with | ||
|
||
```bash | ||
$ tdms -h | ||
``` | ||
|
||
You can invoke it directly or call it from a MATLAB script. | ||
We recommend that beginners with MATLAB installed start with the demonstration MATLAB script. | ||
|
||
### To run the demonstration code | ||
|
||
Move into directory [`examples/arc_01`](./examples/arc_01/), | ||
launch MATLAB and run the MATLAB script: | ||
|
||
[`run_pstd_bscan.m`](./examples/arc_01/run_pstd_bscan.m) | ||
|
||
This script will generate the input to the executable, run the executable and | ||
display sample output. | ||
|
||
### To run standalone | ||
|
||
You can also run `tdms` from the command line... | ||
|
||
```bash | ||
$ tdms --help | ||
Usage: | ||
tdms [options] infile outfile | ||
tdms [options] infile gridfile outfile | ||
Options: | ||
-h: Display this help message | ||
-fd, --finite-difference: Use the finite-difference solver, instead of the pseudo-spectral method. | ||
-q: Quiet operation. Silence all logging | ||
-m: Minimise output file size by not saving vertex and facet information | ||
You can check that `tdms` was installed correctly and is in your `PATH` by running: | ||
```{sh} | ||
tdms --help | ||
``` | ||
in a new terminal. | ||
|
||
The basic workflow is with two arguments; an input file containing source fields, material composition, material properties and other simulation parameters as detailed in the documentation, and an output file name to be created. | ||
The [`iterate_fdtd_matrix.m`](./tdms/matlab/iteratefdtd_matrix.m) script can be used to produce a valid input file from a MATLAB script and source-field functions. | ||
## How to run | ||
|
||
You can choose two possible solver methods: either pseudo-spectral time-domain (PSTD, the default) or finite-difference (FDTD, with option `--finite-difference`). | ||
You can run TDMS either directly or from a MATLAB script. | ||
For beginners, we recommend starting with the demonstration MATLAB script, which you can find in the `examples/arc_01` directory. | ||
Move into this directory, launch MATLAB, and run the MATLAB script [`run_pstd_bscan.m`](https://github.com/UCL/TDMS/blob/main/examples/arc_01/run_pstd_bscan.m). | ||
This script will generate the input to TDMS, run TDMS, and display sample output. | ||
|
||
### Parallelism | ||
If you want to run TDMS standalone at the command line, the basic operation is with two arguments: an input file containing simulation parameters, and an output file name. | ||
You can choose between two solver methods: pseudo-spectral time-domain (PSTD, the default) or finite-difference (FDTD, with option `--finite-difference`). | ||
|
||
TDMS is parallelised with [OpenMP](https://en.wikipedia.org/wiki/OpenMP). The maximum | ||
number of threads can be set with the `OMP_NUM_THREADS` environment variable. | ||
For example, to use 4 threads, in a bash shell, use: | ||
TDMS is parallelized with [OpenMP](https://en.wikipedia.org/wiki/OpenMP). | ||
You can set the maximum number of threads using the `OMP_NUM_THREADS` environment variable before calling the TDMS executable. | ||
|
||
```bash | ||
$ export OMP_NUM_THREADS=4 | ||
```{sh} | ||
export OMP_NUM_THREADS=4 # for example | ||
``` | ||
|
||
Before calling the `tdms` executable. | ||
|
||
## Citation | ||
|
||
If you used TDMS in your research and found it helpful, please cite this work. | ||
|
@@ -131,7 +102,7 @@ If you used TDMS in your research and found it helpful, please cite this work. | |
@software{tdms, | ||
author = {Munro, Peter and others}, | ||
license = {GPL-3.0}, | ||
title = {{TDMS - Time Domain Maxwell Solver}}, | ||
title = {{TDMS - The Time-Domain Maxwell Solver}}, | ||
URL = {https://github.com/UCL/TDMS} | ||
} | ||
``` | ||
|
@@ -157,4 +128,4 @@ Development of this software has previously benefited from funding from the [Com | |
|
||
## Want to contribute? | ||
|
||
We're grateful for bug reports, feature requests and pull requests. Please see our [contribution guidelines](https://github-pages.ucl.ac.uk/TDMS/md__c_o_n_t_r_i_b_u_t_i_n_g.html). | ||
We're grateful for bug reports, feature requests, and pull requests. Please see our [contribution guidelines](https://github-pages.ucl.ac.uk/TDMS/md__c_o_n_t_r_i_b_u_t_i_n_g.html) (we also have some [developer documentation](https://github-pages.ucl.ac.uk/TDMS/md_doc_developers.html)). |
Oops, something went wrong.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So also to note: we could add a default assignee here. I opted not to because I get notifications for all activity (so critical bugs shouldn't go under the radar).
We can set this up for Peter at handover too.