Added timestep check #111
Added timestep check #111
Conversation
There was a problem hiding this comment.
@torbjornstoro I can't request changes because I opened the PR but some suggestions :)
There was a problem hiding this comment.
@torbjornstoro some minor comments , but more importantly, tests are failing 🙏
https://github.com/esa/NIDN/runs/7661751847?check_suite_focus=true
| wavelength (float): wavelength of the simulation | ||
| permittivity (tensor): tensor of relative permittivities for each layer | ||
| """ | ||
| wavelengths_of_steady_state_signal = 5 |
There was a problem hiding this comment.
could you add a comment? I don't get what you mean? the number of wavelengths isn't fixed after all? Do you mean the number of oscillation cycles we want in the signal? Then maybe target_nr_of_steady_cycles or so would be better?
| permittivity (tensor): tensor of relative permittivities for each layer | ||
| """ | ||
| wavelengths_of_steady_state_signal = 5 | ||
| number_of_internal_reflections = 3 |
There was a problem hiding this comment.
Also comment here, this is an assumption I guess? Would it be different for more layers, thicker layers e.g.?
| ) | ||
| + wavelengths_of_steady_state_signal * wavelength / UNIT_MAGNITUDE | ||
| ) | ||
| * torch.sqrt(torch.tensor(2.0)) |
There was a problem hiding this comment.
where does the sqrt(2) come from?
- some minor refactoring - fixing tests, removing unused imports
Description
Summary of changes
Note: Merge #110 first and then switch this one to merge into main.
Resolved Issues
N/A
How Has This Been Tested?