search

modelica / ModelicaStandardLibrary

Free (standard conforming) library to model mechanical (1D/3D), electrical (analog, digital, machines), magnetic, thermal, fluid, control systems and hierarchical state machines. Also numerical functions and functions for strings, files and streams are included.
https://doc.modelica.org
BSD 3-Clause "New" or "Revised" License
481 stars 169 forks source link

New spikes in RectifierCenterTap2mPulse reference results #3711

Open henrikt-ma opened 3 years ago

henrikt-ma commented 3 years ago

The 3.2.3 and 4.0.0 reference results for these models differ due to spikes in rectifier.thyristor_p.idealThyristor[3].off appearing in the 4.0.0 versions:

In both versions, there's a single spike at time 1.667e-3:

idealThyrstor3_off

When simulated using System Modeler, I get the same result as in the 3.2.3 versions (that is, without those spikes), which makes me wonder if those spikes really are a new and expected feature?

henrikt-ma commented 3 years ago

The spike is also seen for this model:

AHaumer commented 3 years ago

Yes these spikes are new, and no this is not an expected feature. To be honest, normally I'm not looking at thyristor_x[n].off since this is only an auxilliary variable. The only reason for not making it protected is to enable proper initialization of the thyristors. These spikes don't affect the results an engineer is looking at, i.e. current and voltages. It stems from infinitesimal numerical inaccuracies. For 4.0.0 I've made PowerConverters.ACDC.Control.Signal2mPulse easier to read. Here we start a timer at a zero crossing of the corresponding voltage, then comparing the timer with respect to the firing angle. The difference is:

3.2.3 timer.y > firingAngle.y/(2*pi*f)
4.0.0 timer.y*2*f > firingAngle.y/pi

We could switch back to the old implementation, but I think it isn't worth to do so.

henrikt-ma commented 3 years ago

How about simply excluding the thyristor_x[n].off from the comparison signals?

AHaumer commented 3 years ago

I don't know why these signal are in the ComparisonSignals.txt ... I'll create a PR. Is it ok to create the PR for both #3711 and #3713 ? Both have the same bugfix, both are for PowerConverters,

maltelenz commented 3 years ago

@AHaumer I see no problem with a single PR to remove comparison signals for both issues.

maltelenz commented 8 months ago

I don't know why these signal are in the ComparisonSignals.txt ... I'll create a PR.

@AHaumer It seems the PR hasn't been created, maybe now is a good time, so we can get the fixes for the reference results in 4.1.0?