Qucs / qucs

Qucs Project official mirror
http://qucs.sourceforge.net/
GNU General Public License v2.0
1.16k stars 213 forks source link

Use left recursion for parsing S-parameters files #791

Closed in3otd closed 6 years ago

in3otd commented 6 years ago

Fix #789. Seems to work fine on all the S parameters file I have at hand here (from 2 to 40 ports and size up to 100+ MB).

Kathy626 commented 6 years ago

what is the maximum file size you can upload after this been fixed?

in3otd commented 6 years ago

I've done some test to try to find that out: I've generated S-parameters files from 1 to 100 ports with 1000 frequency points each and tried to convert them using qucsconv. The original (current) version stops with memory exhausted with 43 or more ports, the version with this fix can successfully convert the 100 port file (which is almost 200 MB in size). I'm not sure which is actually the maximum size then but since now the parser needs to keep on its stack only one line at a a time instead of the whole file as before I guess you could in theory parse files with data for several hundreds of ports :grin:. The fixed code is as fast (or as slow) as before, the time needed to convert the file increases quadratically with the number of ports, not surprisingly:

image

Kathy626 commented 6 years ago

After I modified code, and make all , somehow I got error make all /bin/sh ../build-aux/ylwrap parse_touchstone.ypp y.tab.c parse_touchstone.cpp y.tab.hecho parse_touchstone.cpp | sed -e s/cc$/hh/ -e s/cpp$/hpp/ -e s/cxx$/hxx/ -e s/c++$/h++/ -e s/c$/h/y.output parse_touchstone.output -- bison -y -d -t /home/usr/Documents/qucs-0.0.19/qucs-core/src/parse_touchstone.ypp:110.15-22: error: symbol Dataline is used, but is not defined as a token and has no rules /home/usr/Documents/qucs-0.0.19/qucs-core/src/parse_touchstone.ypp:112.27-28: error: $2 of ‘Dataset’ has no declared type /home/usr/Documents/qucs-0.0.19/qucs-core/src/parse_touchstone.ypp:114.32-33: error: $2 of ‘Dataset’ has no declared type /home/usr/Documents/qucs-0.0.19/qucs-core/src/parse_touchstone.ypp:115.23-24: error: $2 of ‘Dataset’ has no declared type make: *** [parse_touchstone.cpp] Error 1 This is really weird

in3otd commented 6 years ago

yep, that's really strange, the build here is fine and also Travis and AppVeyor show no issues. Did you pull the branch were the changes were made? (but there's just ~one~ Edit: three changed files). BTW, for Windows you can get ready-made binaries for this PR here.

Kathy626 commented 6 years ago

I cheatted, I didn't use GitHub branch pull, instead, I have went into the Source Code for modification. But I don't think that matters?! OR does it master a lot? As you mentioned there is one changed file, but I saw three changed file, they are check_touchstone.cpp and check_touchstone.h, and parse_touchstone.ypp. So did you only changed .ypp file and the other two is automake?

in3otd commented 6 years ago

yep, the changed files are three actually, I corrected my comment above - I was thinking just at the one with the most changes. In any case, if you modified the files manually it should of course work fine.