HomerReid / scuff-em

A comprehensive and full-featured computational physics suite for boundary-element analysis of electromagnetic scattering, fluctuation-induced phenomena (Casimir forces and radiative heat transfer), nanophotonics, RF device engineering, electrostatics, and more. Includes a core library with C++ and python APIs as well as many command-line applications.
http://www.homerreid.com/scuff-em
GNU General Public License v2.0
126 stars 50 forks source link

Problems tokenizing potential file for scuff-static on Windows #54

Open sarahdh opened 9 years ago

sarahdh commented 9 years ago

I am running scuff-static on Windows via Cygwin.

The command I am running is: ./scuff-static.exe --Geometry testcube.scuffgeo --EPFile testcube.ep --PotentialFile testcube.potential

This creates the file scuff-static.log, which ends with "received signal 11" -- Google says this means seg fault. The process ends without printing "Thank you for your support." (which, based on looking at the code, it seems like any successful execution should print). It does not create a testcube.out file.

I have determined that this issue is stemming from the following function in libhrutil.cc:

int Tokenize(char *s, char **Tokens, int MaxTokens, const char *Separators)

In particular, things seem to work fine if I remove the following lines from the code:

while ( *saveptr && strchr(Separators, *saveptr) )
   saveptr++;
if (*saveptr)
   Tokens[NumTokens++]=saveptr;

(Though, of course, there are many ways in which my potential file could be a corner case -- for example, it only has a single line that needs to get parsed.)

So my main question is: How vital is this part of the code? The following snippet appears earlier in the function:

(void) saveptr; // unused

Is saveptr really unused? Is it fine to remove the saveptr while loop + if statement from the code?

A secondary question is: I'm trying to read testcube.out, but I don't know what format I'm expecting it to be in. On each line, I recognize the first three numbers as the x, y, and z coordinates of the point, but there are four other numbers on each line -- what meaning do they have? Is the seventh one the potential?

Thanks!

HomerReid commented 9 years ago

First off, regarding the .out file produced by SCUFF-STATIC, it was an oversight on my part that the code failed to write an informative output-file header to allow the user to make sense of the data. I have rectified that oversight in 7733e61c963c5f1921980f8bce6e9a0ad7030d13. Thanks for pointing this out! In the meantime, the long story short is that

columns 1,2,3 are the coordinates of the evaluation point (as you observed) column 4 is the electrostatic potential columns 5,6,7 are the components of the electrostatic field (Ex, Ey, Ez)

Next, regarding the curious behavior of Tokenize on Windows: First, thanks for taking the time to track down the origin of the problem, which makes things 1000x easier for me. Looking at the code, it seems the (void) saveptr; snippet was added inside an #ifdef (_WIN32) ..#endif block, presumably to fix some other error on Windows systems, and my suspicion is that this line is causing the problem, since (as you observe) the variable in question is, in fact, not unused. I have gone ahead and removed this line from the code in the latest repository commit referenced above. Hopefully the core dump you are experiencing will go away with this, and fingers crossed that this doesn't introduce some other byzantine errors on Windows.

Thanks for this feedback. Feel free to keep the discussion going here, or close the issue if it has been resolved to your satisfaction.

sarahdh commented 9 years ago

Thanks for the code updates. The .out header was exactly what I was looking for, but the other change doesn't seem to have affected anything. That is, the code still works if and only if I remove this:

while ( *saveptr && strchr(Separators, *saveptr) )
   saveptr++;
if (*saveptr)
   Tokens[NumTokens++]=saveptr;
HomerReid commented 9 years ago

Looking more closely at the code, it seems the issue arises from the fact that Windows (or at least MinGW) doesn't support the thread-safe version of strtok, so the code had been punting by using the non-thread-safe version on Windows installations. This probably dates back to an earlier era in which there were no calls to Tokenize() in multithreaded code sections, but nowadays there are, so the punt doesn't work anymore.

I found an open-source thread-safe version of strtok and put it into Tokenize() when compiling on Windows. This fix is implemented in 6e4297d454f5b4142707587da018b49e22508e23. Please check and close the issue if this fixes the problem.

sarahdh commented 9 years ago

Problem is same as before, despite this change. Value of saveptr is 0x0.