flaport / fdtd

A 3D electromagnetic FDTD simulator written in Python with optional GPU support
https://fdtd.readthedocs.io
MIT License
455 stars 116 forks source link

new grid permittivity strategy & issues with PML and Objects #6

Closed tlambert03 closed 4 years ago

tlambert03 commented 4 years ago

I fear I may have caused more problems in issue #4. It appears that commit 3e22a26 has introduced some new bugs relating to PMLs and certain objects. (either that or I'm using them incorrectly, and this commit exposed my errors)

To explain the issue, let me describe my simulation. I have a simply "TIR" example grid where the top half has a higher refractive index (1.55) than the bottom (1.33), an angled LineSource points at the middle of the grid, and a (somewhat strange) very high refractive index "absorbing" object serves to block stray energy on the left side at the interface, and PMLs surround the grid (note: the 1.55 RI region overlaps the PML):

grid layout

previously, this was the result after running for a while:

old_version

these lines in commit 3e22a26 now cause some strange (high magnitude E-field) results in both the PML and my weird high refractive index object at the interface:

new version

I can remove the boundary errors by taking extra care not to have any objects overlapping the PML regions, (though previously, fdtd handled this a bit more gracefully, and it didn't matter whether these overlapped). However, I still have problems with my "absorbing" object at the interface.

new_without_overriding_pml

it's possible that I'm just not implementing absorbing objects correctly, or that the object edge code just needs some tweaks ... but it also seems like there are generally more "gotchas" now with the new grid permittivity strategy and the PML than there were before you generously dealt with my request to remove objects :)

I tried for a while to figure out a PR to fix it gracefully while leaving the ability to remove objects, but so far have failed. I personally wouldn't mind if you wanted to rollback the grid removal idea for a while, if it's the easiest way to deal with this. I know you're busy with other things!

thanks again!

flaport commented 4 years ago

The previous commit was rolled back. Thanks for noticing! I'll see if I can find a better solution in the near future. Also, the library clearly needs some tests (#7) to prevent these unexpected bugs in the future.

I haven't figured out what exactly caused this numerical instability, so I'll leave this issue open until either I find the problem or find a better way of implementing the removal of the objects.

tlambert03 commented 4 years ago

Sounds like a plan. thanks again. The object removal request is certainly not that pressing, especially given the ease of simply making a new grid (though I suppose someone might eventually want to simulate the removal of an object after creating some grid state?).

I might be able to contribute some tests. do you have a preferred test runner? (I can also just keep an eye out for you to create a few tests first and then adhere to your style)

flaport commented 4 years ago

My preferred test runner is pytest. I created some sample test cases, feel free to add some more. Read #7 for some guidelines and discussion about tests. I'm closing this issue for now since I most likely won't go back to the solution (#6) initially proposed in #4, which solves this issue. The object removal discussion can be had there.