search

thliebig / openEMS

openEMS is a free and open-source electromagnetic field solver using the EC-FDTD method.
http://openEMS.de
GNU General Public License v3.0
427 stars 150 forks source link

diagonal voltage integration and voltage dump boxes #70

Closed boomboompsh closed 3 years ago

boomboompsh commented 4 years ago

The voltage dump box is very hacky and un-optimized, though without changing the interface or doing a lot of math, it's the best I could do and good enough for what I need. It's not fully tested, I've only run the microstrip notch filter simulation with it, but it does produce pretty pictures. image

thliebig commented 4 years ago

Hi, interesting idea and I like it that somebody is looking at the C++ core and modifies it. But I have a few comments and recommendations. I think 20c3b26 is trivial and just fine. About 26bffd8 I am wondering why you changed it to a diagonal interpolation. EM theory says it should not make any difference which path you take but in numeric results that may be slightly different. Did you see any indications for a problem? What I do not like is changing that method for everyone e.g. probes. If you really need the diagonal version for your new dump type, I recommend adding it as a new method and name it e.g. CalcVoltageIntegralDiagonal. About your main new dump type. There a few things I would recommend to do differently: First, why are you doing it in the core at all? Why not just dump the (uninterpolated) electric field and then do the voltage calculating in Matlab/Octave/Python? It could be much more flexible and easily implemented in an even much more efficient way. Which brings be to point two. Using the integral function is not a good idea as this is numerically very expensive to do for every point in space. For large dumps this will scale horribly. This could be done in a much easier and much faster way (see first point again) because for every new point you could just use its neighbor and add the E*dl to it and would only need to go through every point once. Should be fast and easy to do in Matlab etc. What do you think, regards Thorsten

boomboompsh commented 4 years ago

I'm doing it the way that I am because I couldn't figure out the VTK API. (In no small part because the website went down while I was trying to learn it). Which is probably a bad reason, but at least it's a compliment to your code that I found it easier to modify it than to use a documented API. I changed how the voltage Integral works because what it did before gave incorrect results if a voltage wasn't measured along an axis, before changing it, my voltage graph didn't change at all in the x direction. The bug was as follows:

image

This is the path a line integral should take. The integral of the segment going up is added to the segment going right.

image

This is the path CalcVoltageIntegral was taking. It starts over from the start point each time, instead of following the path.

I don't expect this to be pulled, In large part I'm just proud I managed to get a modification working in an unfamiliar code base in a language I haven't used in years, and wanted to show off.

Another problem with my code you didn't mention is that there's no way to specify the voltage reference for the measurement, it always goes off the first point in the dump box.

thliebig commented 4 years ago

Looking at the voltage integral again I think you are right, it is not doing it right. I guess it never was a problem as all ports are only line, but I will need to check. If that is indeed the case, changing it to be diagonal or at least correctly Cartesian would be a good option. But as it is I am not going to accept this PR as I still think doing this voltage dump in the core is the wrong approach. Did you see this matlab function? Dump2VTK With this you can easily dump a scalar field to a vtk file after you have done your calculation in Matlab using a simple hdf5 E-field dump. And you would be able to give it the missing voltage reference too.

boomboompsh commented 3 years ago

I have removed the v-field dump box code to leave just the bugfix for the voltage integration. All tests pass except the fieldprobes, but they don't pass even on the unmodified code, so that's probably not a problem. I think it's relatively important to fix this bug, since users can create voltage probes manually with AddProbe, and those give incorrect results with the current code. I can do more testing if needed. Sorry for the long wait.

thliebig commented 3 years ago

I had a closer look at your code today and I think the changes to the start>stop case is not correct. Assume start=0 and stop=3: Iteration is over pos in 0,1,2 with positive sign. All good. Now the case start=3 and stop=0. This must give the same result just with a negative sign. It used to give the same iteration just with -1 Now you iterate: pos in 3,2,1 That is not correct... Can you create a fixed patch? Furthermore I should create a test-file to validate this I guess...

thliebig commented 3 years ago

By the way if you need more details: start and stop are node indices, e.g. from Node 0 to Node 3. But the voltage integration has to be done along the edges from this nodes. Edge 0 goes from Node 0 to Node 1. Thus we must iterate over the edges 0,1,2 and edge 3 must not be included. Additional difficulties arise if the integration path is not just in one direction. I have to think about this a bit more..

thliebig commented 3 years ago

I checked it, the case start<stop is correct in your patch, you just need to treat the case start>stop the same way just treat start as stop and vice versa (same integration path) but with a minus sign

thliebig commented 3 years ago

Well today I looked much deeper into this and realized that what I said in my first post is actually not true for frequencies above 0. The electric field for RF frequencies is not conservative. Which means the path you integrate along does matter and always gives different results depending on the changing magnetic flux you enclose (or not enclose). That means that a voltage plot like you proposed initially is not possible and makes no physical sense and you should not use this approach for anything meaningful!

This also means integrating along a diagonal does not make any sense as multiple Cartesian paths would be conceivable but all would yield different results.

I have started working on this topic and the voltage probe and voltage integration, but the current patches of this PR are not correct and thus I will just close this PR. But thanks for the input and discussion, it did help to improve openEMS !