Simulation plot GUI slow with short simulation step duration

Bug #1674143 reported by Imre Deak on 2017-03-19
14
This bug affects 2 people
Affects Status Importance Assigned to Milestone
KiCad
Low
Hal Gentz

Bug Description

Application: kicad
Version: (2017-03-18 revision 98e3bfb)-master, release build
Libraries: wxWidgets 3.0.2
           libcurl/7.50.1 GnuTLS/3.5.3 zlib/1.2.8 libidn/1.33 librtmp/2.3
Platform: Linux 4.10.0-rc8+ x86_64, 64 bit, Little endian, wxGTK
- Build Info -
wxWidgets: 3.0.2 (wchar_t,wx containers,compatible with 2.8)
Boost: 1.61.0
Curl: 7.50.1
KiCad - Compiler: GCC 6.2.0 with C++ ABI 1010
        Settings: USE_WX_GRAPHICS_CONTEXT=OFF
                  USE_WX_OVERLAY=OFF
                  KICAD_SCRIPTING=ON
                  KICAD_SCRIPTING_MODULES=OFF
                  KICAD_SCRIPTING_WXPYTHON=ON
                  KICAD_SCRIPTING_ACTION_MENU=OFF
                  BUILD_GITHUB_PLUGIN=ON
                  KICAD_USE_OCE=OFF

The plot rendering and cursor zoom-in responsivity is unusably slow for a 1s simulation with 1us step duration. The corresponding Spice .listing is below. I'm interested in using such short step durations since it seems to be the only way I can get accurate results. I also tried to use the Spice max_step parameter in the .tran control OP like:

.tran 1m 1 0 1u

but it didn't seem to make a difference.

Circuit: KiCad schematic
KiCad schematic
1 : kicad schematic
2 : .global gnd
5114 : .model pn2222a npn (is=14.34f xti=3 eg=1.11 vaf=74.03 bf=255.9 ne=1.307 ise=14.34f ikf=.2847 xtb=1.5 br=6.092 nc=2 isc=0 ikr=0 rc=1 cjc=7.306p mjc=.3416 vjc=.75 fc=.5 cje=22.01p mje=.377 vje=.75 tr=46.91n tf=411.1p itf=.6 vtf=1.7 xtf=3 rb=10)
5138 : .model pn2907 pnp (is=650.6e-18 xti=3 eg=1.11 vaf=115.7 bf=231.7 ne=1.829 ise=54.81f ikf=1.079 xtb=1.5 br=3.563 nc=2 isc=0 ikr=0 rc=.715 cjc=14.76p mjc=.5383 vjc=.75 fc=.5 cje=19.82p mje=.3357 vje=.75 tr=111.3n tf=603.7p itf=.65 vtf=5 xtf=1.7 rb=10)
5428 : vv1 net-_q2-pad3_ 0 dc 8
5429 : rr1 net-_q2-pad3_ npnb 63k
5430 : rr2 npnb 0 5.6k
5431 : rr4 vout 0 200
5432 : cc1 vout npnb 22u
5433 : qq1 pnpb npnb 0 pn2222a
5434 : qq2 vout pnpb net-_q2-pad3_ pn2907
5435 : .save @vv1[i]
5436 : .save @rr1[i]
5437 : .save @rr2[i]
5438 : .save @rr4[i]
5439 : .save @cc1[i]
5440 : .save @qq1[ib]
5441 : .save @qq1[ic]
5442 : .save @qq1[ie]
5443 : .save @qq2[ib]
5444 : .save @qq2[ic]
5445 : .save @qq2[ie]
5446 : .save v(0)
5447 : .save v( 0 )
5448 : .save v(net-_q2-pad3_)
5449 : .save v(vout)
5450 : .save v(npnb)
5451 : .save v(pnpb)
5452 : .control
5453 : listing
5454 : .endc
5455 : .tran 1u 1
5457 : .end
Background thread stopped with timeout = 0
Doing analysis at TEMP = 27.000000 and TNOM = 27.000000
Initial Transient Solution
--------------------------
Node Voltage
---- -------
net-_q2-pad3_ 8
npnb 0.631761
vout 7.88666
pnpb 7.17048
vv1#branch -0.040181
 Reference value : 0.00000e+00
 Reference value : 1.41922e-02
 Reference value : 2.88152e-02
 Reference value : 4.35692e-02
 Reference value : 5.79132e-02
 Reference value : 7.16722e-02
 Reference value : 8.45952e-02
 Reference value : 9.70152e-02
 Reference value : 1.10691e-01
 Reference value : 1.24261e-01
 Reference value : 1.36660e-01
 Reference value : 1.48216e-01
 Reference value : 1.59004e-01
 Reference value : 1.69720e-01
 Reference value : 1.80509e-01
 Reference value : 1.90200e-01
 Reference value : 1.99672e-01
 Reference value : 2.13170e-01
 Reference value : 2.27611e-01
 Reference value : 2.41918e-01
 Reference value : 2.56504e-01
 Reference value : 2.71148e-01
 Reference value : 2.84939e-01
 Reference value : 2.99105e-01
 Reference value : 3.13070e-01
 Reference value : 3.27282e-01
 Reference value : 3.41380e-01
 Reference value : 3.55643e-01
 Reference value : 3.70269e-01
 Reference value : 3.84761e-01
 Reference value : 3.98641e-01
 Reference value : 4.13495e-01
 Reference value : 4.28258e-01
 Reference value : 4.43248e-01
 Reference value : 4.57897e-01
 Reference value : 4.72324e-01
 Reference value : 4.86992e-01
 Reference value : 5.01648e-01
 Reference value : 5.16285e-01
 Reference value : 5.30989e-01
 Reference value : 5.45721e-01
 Reference value : 5.60387e-01
 Reference value : 5.75004e-01
 Reference value : 5.89703e-01
 Reference value : 6.04436e-01
 Reference value : 6.19102e-01
 Reference value : 6.33699e-01
 Reference value : 6.48194e-01
 Reference value : 6.62910e-01
 Reference value : 6.77677e-01
 Reference value : 6.92429e-01
 Reference value : 7.07143e-01
 Reference value : 7.21589e-01
 Reference value : 7.35744e-01
 Reference value : 7.49895e-01
 Reference value : 7.63722e-01
 Reference value : 7.77554e-01
 Reference value : 7.91048e-01
 Reference value : 8.05140e-01
 Reference value : 8.19687e-01
 Reference value : 8.34215e-01
 Reference value : 8.48809e-01
 Reference value : 8.64113e-01
 Reference value : 8.79244e-01
 Reference value : 8.94093e-01
 Reference value : 9.08645e-01
 Reference value : 9.23023e-01
 Reference value : 9.37640e-01
 Reference value : 9.52231e-01
 Reference value : 9.67017e-01
 Reference value : 9.81742e-01
 Reference value : 9.96533e-01
No. of Data Rows : 1000088

Tomasz Wlostowski (twlostow) wrote :

Hi Imre,

Could you send us the circuit that causes the issue?

Best,
tom

Imre Deak (ideak) wrote :

Yes, I attached the archived KiCad project.

For quick reference it's the following circuit:
http://electronics.stackexchange.com/questions/292901/capacitor-polarity-in-lamp-flasher

tags: added: spice
Seth Hillbrand (sethh) wrote :

The attached project does not appear to be particularly slow with current master. is this still an issue?

Changed in kicad:
importance: Undecided → Low
status: New → Incomplete
Launchpad Janitor (janitor) wrote :

[Expired for KiCad because there has been no activity for 60 days.]

Changed in kicad:
status: Incomplete → Expired
Hal Gentz (gentz) wrote :

Drawing and redrawing the simulation gui is also slow for me when the time step is low. For me, I experience this issue when timestep = "10ns" and end = "20ms", having to wait around 28 seconds between repaints! The schematic I used attached bellow.

What's happening is, that these small time steps increas the number of data points we have iterate over when drawing the graph. With the above config, there are roughly 2 million data points. `mpFXY`'s `Plot` function gets called every (re)draw, which (re)iterates over every single one of those data points once for each line, which of course takes a while. (See lines 700 to 742 in `common/widgets/mathplot.cpp`)

Now, since the actual graph's size is pretty damn small, most of these data points are just drawing lines from and to the same pixels. So I've come up with a "caching" system, where when we first draw a layer, we store all the unique draw calls it issued, then on every redraw we don't reiterate over the 2 million data points and instead just issue the same draw calls as that first draw.

Of course, the first draw still takes 28 seconds or so, and if you try to zoom in/resize the application, ect, you'll have to re wait those 28 seconds. So, this patch only makes it bearable, not actually fixing the root problem. Is there a proper fix out there? Who knows?

Hal Gentz (gentz) wrote :
Changed in kicad:
status: Expired → New
Hal Gentz (gentz) wrote :

Version info:
Application: eeschema
Version: (6.0.0-rc1-dev-574-g030922827-dirty), release build
Libraries:
    wxWidgets 3.0.4
    libcurl/7.61.1 OpenSSL/1.1.1 zlib/1.2.11 libidn2/2.0.5 libpsl/0.20.2 (+libidn2/2.0.4) nghttp2/1.33.0
Platform: Linux 4.18.9.a-1-hardened x86_64, 64 bit, Little endian, wxGTK
Build Info:
    wxWidgets: 3.0.4 (wchar_t,wx containers,compatible with 2.8) GTK+ 2.24
    Boost: 1.68.0
    OpenCASCADE Technology: 7.3.0
    Curl: 7.61.1
    Compiler: GCC 8.2.1 with C++ ABI 1013
Build settings:
    USE_WX_GRAPHICS_CONTEXT=OFF
    USE_WX_OVERLAY=OFF
    KICAD_SCRIPTING=ON
    KICAD_SCRIPTING_MODULES=ON
    KICAD_SCRIPTING_WXPYTHON=OFF
    KICAD_SCRIPTING_ACTION_MENU=ON
    BUILD_GITHUB_PLUGIN=ON
    KICAD_USE_OCE=OFF
    KICAD_USE_OCC=ON
    KICAD_SPICE=ON

Maciej Suminski (orsonmmz) wrote :

Hi Hal,

Thank you for investigating the issue, surely the plot panel leaves space for performance improvements.

I have tested your patch, but have not noticed much difference in speed. I read the patch and agree that it should improve the situation a bit, but instead I suggest decimating drawing points. The simple approach would keep calling GetNextXY() and TransformToPlot() until X coordinate increments and use the mean value of the points that have been skipped to determine Y coordinate. I may implement it next week, unless you want to handle it.

Hal Gentz (gentz) wrote :

Hi Maciej,

Decimating the number of drawed points does sound as a good idea, however using the mean Y coordinate might result in some weird visual quirks. E.g. If one of the lines switches rapidly between, lets say, -5V and 5V, it might get rendered as a straight line at 0V instead of as a thick bar between -5V and 5V.

I suggest that we instead store the min and max Y for each X coordinate, and draw two lines for each X coordinate. I think this would end up being a good comprise between performance and visual quality.

I'll probably take a swing at this sometime today or tomorrow.

Hal Gentz (gentz) wrote :

I found that we still have to call `x2p` and `y2p` to figure out which pixels the coordinates are on. Anyways, I instead decided to disable the `DrawLine` calls inside that loop and instead I have them use the cache which I generate. Because the cache is an std::set, all the draw calls now will be unique. This has sped up the first graph draw considerably for me.

Can you take a look at this patch Maciej? Hopefully you can experience the same perf boost it's giving me.

Maciej Suminski (orsonmmz) wrote :

Storing min/max Y values for each X coordinate is a very good idea.

I have tried the new patch, but I have not noticed much difference. On the other hand, plotting on my PC does not take that much time, so I cannot really say your changes does not help. Still, I opt for the decimation approach, so could you test the attached patch? It is just a proof of concept, still lacks your idea of drawing both min and max Y values for each X coordinate. If it solves the performance problem, then I may add the missing part soon.

Hal Gentz (gentz) wrote :

+1 It works wonderfully.

Sadly, my green line, instead of being a straight bar has become bumpy, I think it's the average issue I mentioned. See this pic: https://i.imgur.com/3n6cfTc.png

I think we will have to store min and max Y values for each X coordinate for continuous plots, and every single Y value for non continuous plots. I've implemented this in the variant of your patch attached bellow.

However, I've noticed some weird artifacts when zooming in, and I think those will need investigation.

Hal Gentz (gentz) wrote :
Maciej Suminski (orsonmmz) wrote :

Your changes seem fine to me. Can you say more about the artifacts? Please give credentials that I can use for committing the patch, once we resolve the artifacts issues.

Hal Gentz (gentz) wrote :

Sorry for the late response.

The problem I was experiencing was that when I zoomed in too far, the lines disappeared, and zooming out took you somewhere random. Appears that these problems are present even before my patch was applied, so I guess they are a separate bug.

What credentials do you need?

Maciej Suminski (orsonmmz) wrote :

No problem, in such case I will merge the patch. Regarding credentials, I suppose I can use "Hal Gentz" as the author name, then I need just your e-mail address.

Hal Gentz (gentz) wrote :

My email is "<email address hidden>".

Maciej Suminski (orsonmmz) wrote :

Thank you Hal, I pushed yours and mine changes to the master branch. BTW, if you use git for development, git format-patch is the preferred way of creating patches. It makes our lives sooo much easier.

Changed in kicad:
milestone: none → 5.1.0
KiCad Janitor (kicad-janitor) wrote :

Fixed in revision 416e64a33419cc33c8cd37dbd1e1155bb0b6efc5
https://git.launchpad.net/kicad/patch/?id=416e64a33419cc33c8cd37dbd1e1155bb0b6efc5

Changed in kicad:
status: New → Fix Committed
assignee: nobody → Hal Gentz (gentz)
Hal Gentz (gentz) wrote :

Ah, I was using git diff, will do next time. Thanks, Maciej.

Changed in kicad:
status: Fix Committed → Fix Released
To post a comment you must log in.
This report contains Public information  Edit
Everyone can see this information.

Other bug subscribers