Re: [Gnucap-devel] Fwd: common_component and eval_bm_acton_base

[al davis]

On Saturday 06 November 2010, al davis wrote:
> I have one question regarding elaboration of , for example
> eval_semi_resistor - semiconductor resistor.
> 
> As I understand, when resistor is parsed - COMPONENT is
> created and COMMON_COMPONENT (namely - EVAL_BM_MODEL object)
> is set in COMPONENT::_common.

EVAL_BM_MODEL is a placeholder for things like 
EVAL_SEMI_RESISTOR and others.  Which one to use cannot be known 
until elaboration (expand), when it finds the MODEL.

EVAL_BM_MODEL with something attached at _func is equivalent to 
whatever is attached at _func, substituted, but with one extra 
level of indirection.  "deflate" removes extra indirections.

The only information in EVAL_BM_MODEL is the string with 
parameters, which is parsed and interpreted during expand.  
Parameters in COMPONENT_COMMON (base class) are not used in the 
placeholder, and should never be changed from the defaults.

"obsolete_callback" is old code still around, that existed 
before plugins or parameter expressions were implemented.  A 
rewrite is planned, so it has not been re-tested as thouroughly 
as permanent code.

> later on, during elaboration (namely - during expansion),
> COMPONENT calls expansion for COMMON_COMPONENT,  for
> EVAL_BM_MODEL class it means process like that:
> 
>    EVAL_BM_MODEL:expand()
>     a) finds and calls proper constructor (hierarchy of
>     COMMON_COMPONENT), depending on the model type
>     of the associated .model, in this case -
>     EVAL_SEMI_RESISTOR
>     b) attaches new common_component to
>     existing (_func)
>     c) prepare for "deflation" -
>     i.e.  substitution of EVAL_BM_MODEL common_componnet with
>     "daughter" EVAL_SEMI_RESISTOR.
>     d) then during deflation - new COMMON_COMPONENT 
>     substitutes old one.
> 
> Issue I have found is that before EVAL_BM_MODEL:expand() all
> COMMON_COMPONENTs were passed already through precalc_first()
> procedure and for many of them some parameters are set.
> Like _temp_c and alike. See
> COMMON_COMPONENT::precalc_first().
> 
> When at stage a) new COMMON_COMPONENT is created - it does
> not contain all that information, it is lost.

There should be no information there.

> I suppose tis behavior is notfully correct - as
> un-initialized values are passed to calculations and it may
> cause issue later - so it likely has to be fixed.

No ..   It is parsed again:
  CS args(CS::_STRING, _arglist);
  c->parse_common_obsolete_callback(args);

> There are a few ways to fix that:

(I changed the order.)

> 2) explicitly copy parameters between objects, creating
> methods like - copy_params or alike

The parameters of EVAL_SEMI_RESISTOR (and others) do not exist 
as parameters in EVAL_BM_MODEL, because it cannot be known then 
what they are.  Therefore, the whole string is stored, then 
explicitly copied, actually reinitialized, by 
parse_common_obsolete_callback.

But as you discovered, I missed something:

> 1) move initialization code from precalc_first to
> precalc_last - so initialization will not be lost. simplest

This may appear to work, but the real problem is that 
precalc_first is never called!  So, it is not a general 
solution.  It might hide the problem, but doesn't really fix it.

precalc_first and precalc_last both calculate things that are 
needed before any simulation run.  The reason for the split is 
that some parameters must be calculated before expansion, 
because the proper expansion depends on them, and some other 
parameters must be calculated after expansion because they 
depend on something that happens during expansion, such as model 
attachment.  Most parameters can be in either place, it doesn't 
matter.  Over time, I have gone back and forth on my preference.  
The current preference is that when it seems to not matter, it 
should be done in precalc_first, because it exposes bugs sooner.

What is needed is to call precalc_first.

  c->parse_common_obsolete_callback(args);
  c->precalc_first();
  c->expand(d);

Then precalc_last will be called at the usual time.

This is probably not the only place that calls to precalc_first 
are missed.

> 3) pass original EVAL_BM_MODEL to new_common() in "this" to
> call  new constructors, which will create properly
> initialized objects

This is incorrect because EVAL_BM_MODEL contains no information, 
only placeholders.  The correct values are in the daughter 
object, which would be wiped out when the placeholder is copied 
over it.

> NB:
> I made tests - there are some numeric differences - quite
> significant (more than roundoff error)  - like new 0.99991
> against old 0.99999. so, seems this change have some
> influences onto existing test cases.

Can you post the tests?

al.