Repeating C blocks changes its behaviour

[mosfet]

Hi,

I have created a C block which generates a sawtooth with a delay. I wanted to compare it with and without delay in the same simulation and thus I replicated the circuit:

The strange thing is the output behaves differently depending on whether both C blocks are enabled or not. If both are enabled, the one without delay works properly. If I disable the one without disable, then the one with delay works properly.

What am I missing?

mod_SS_MVP.qsch (4.9 KB)
modulation.cpp (1.4 KB)

Regards

[KSKelvin]

Possibly problem in using //static float timeprev = 0, tdiff;
Here is a quick modification. But the best answer should from @RDunn .

modulation.cpp (2.0 KB)

[RDunn]

Hi, MOSFET.

I didn’t investigate deeply but static local variables are global variables with local namespace.

static float switching_frequency = 135e3;
const float SAFETY_MARGIN = 1.2;
static float timeprev = 0, tdiff;

Globals are evil even when disguised as static locals. Move them to per-instance data structure as described in the first C-Block Basics paper here.

–robert

[mosfet]

Solved by using the internal instance struct! Thank you both.

Regards

[mosfet]

Actually I’m seeing a weird behaviour where just at the very beginning, the first period is not integrated with the correct slope. That happens for both modules. When both are enabled, after the no delay block reaches ‘1’ and resets, then the slope is correct for both. The slope change mid period for the delayed block messes the delay.

modulation.cpp (2.0 KB)
mod_SS_MVP.qsch (3.7 KB)

Any idea why this might be happening?

If only the delayed block is left, the slope change does not happen but the slope in incorrect in the first period:

Regards

[RDunn]

Hi, mosfet.

I think that the clue is in the behavior that you’ve observed.

if (t < delay)
{
inst->timeprev = t;
VM = 0;
 return;
 }

The above prevents reaching the code that actually does the needed calculations until the simulation clock reaches the first delay.

If you move the (evil) static switching_frequency into per-instance data and initialize as in the attached, the kink goes away. You’ll need to initialize other variables to fix the initial slope issue.

–robert

modulation.cpp (2.1 KB)

[mosfet]

Ahh indeed!

Thank you very much.

[erma]

Hey @RDunn I had similar problems in the past which I solved adapting part of the code that @KSKelvin uploaded in his github.

I wonder, how bad is my workaround compared to the solution you proposed above?

struct stateStr{
    bool clk_state;

    stateStr(){
    clk_state = false;
    }
 };

extern "C" __declspec(dllexport) void dac(void **opaque, double t, union uData *data)
{
//inputs and outputs
   ...

// Only once: call constructor
if(!*opaque)
{
   *opaque = new stateStr();
}
stateStr* instP = static_cast<stateStr*>(*opaque);

//Do stuff with instP
...

}

Also, should I add something like

extern "C" __declspec(dllexport) void Destroy(struct stateStr *instP)
{
   free(instP);
}

?

Best,
Ermanno

[RDunn]

Hi, Ermanno.

Well, I was trying to respond with minimal changes to mosfet’s code so I didn’t simplify the code or change it to match my “style”…

You use static_cast which isn’t strictly necessary if you change the passed type from “void** opaque” to “stateStr** opaque”.

Personaly, I also move the instance pointer initialization above the one-time initialization code (so that the pointer can be used inside the one-time initialization code). Then add the instance pointer to the *opaque initialization. My version of your sample looks like this (if I didn’t screw it up; long day, working on fumes here):

struct stateStr{
    bool clk_state;

    stateStr(){
    clk_state = false;
    }
 };

extern "C" __declspec(dllexport) void dac(stateStr **opaque, double t, union uData *data)
{
   //inputs and outputs
   ...

   stateStr* instP = *opaque;

   // Only once: call constructor
   if(!stateStr)
   {
      instP = *opaque = new stateStr();
   }

   //Do stuff with instP
   ...
}

Note: I also always name the per-instance struct “InstData” and the pointer pInst. Just my style choice. Consistency has saved a few of my toes. But, you do you.

As for adding Delete(), you don’t need the “struct” keyword in the function signature and, because the instance was allocated with “new”, I’d release with “delete”. (IIRC free() is technically allowed but sort of bad style.)

Do you need to add Destroy()? Technically, Destroy() is called for each instance at the end of each simulation step. If there are multiple steps, each step calls the evaluation function with null **opaque and you allocate new per-instance space. The old instance heap space isn’t freed if you don’t include Destroy(). All allocated space does eventually get freed when the simulation terminates. But you leak memory until then.

Your example doesn’t allocate much per-instance memory so it’s rather inconsequential. On the other hand, you might eventually do something more complicated that allocates more memory, opens files that need to be freed/closed in a destructor, etc. If you don’t call delete, destructors defined in the per-instance struct don’t get called.

Bottom line: I’d add Destroy() to ensure that future code revisions don’t introduce issues.

So that’s my two cents worth. Hope it helps.

–robert

[erma]

Thank you very much @RDunn, I finally had time to properly test the code in vscode to get a better grasp of it.

There are a couple of things I am confused by your response.

1-

Destroy() is called for each instance at the end of each simulation step.

but in your CBlock_Basics.pdf I read

So, is Destroy() called at the end of each timestep or at the end of the simulation?
2-

If there are multiple steps, each step calls the evaluation function with null **opaque and you allocate new per-instance space.

Did you mean “multiple instances of the .dll block” rather than “multiple steps” (I assumed [time]steps)?

Thanks,
Ermanno

[RDunn]

Hi, Ermanno.

Well, that is confusing, isn’t it.

I’m referring to “simulation steps” in the context of the “.step” SPICE directive. See the QSpice help:

Does that clear it up?

–robert

[erma]

Thanks @RDunn, that clarifies both my points!

Ermanno