From 9786d3b8e2999bada3089b274326ffb5112068c3 Mon Sep 17 00:00:00 2001 From: Giles Atkinson <“gatk555@gmail.com”> Date: Sat, 25 Jan 2025 13:43:47 +0000 Subject: [PATCH] Improve checking of XSPICE model parameters and tidy code. Checking for null values moves from per-instance code to per-device code, potentially removing duplicate error messages. Limits on parameter values and sizes of array parameters that are specified in the IFS files are now actually checked. --- src/xspice/mif/mif_inp2.c | 42 ++----- src/xspice/mif/mifgetmod.c | 132 ++++++++++++++------- src/xspice/mif/mifmpara.c | 229 ++++++++++++++++++++++--------------- 3 files changed, 239 insertions(+), 164 deletions(-) diff --git a/src/xspice/mif/mif_inp2.c b/src/xspice/mif/mif_inp2.c index a45c0059d..34aba5825 100644 --- a/src/xspice/mif/mif_inp2.c +++ b/src/xspice/mif/mif_inp2.c @@ -570,39 +570,23 @@ MIF_INP2A ( } } - /* check model parameter constraints */ - /* some of these should probably be done in MIFgetMod() */ - /* to prevent multiple error messages */ + /* Check model parameter constraints. */ for(i = 0; i < DEVices[type]->DEVpublic.num_param; i++) { + char* emessage = NULL; param_info = &(DEVices[type]->DEVpublic.param[i]); - - if (mdfast->param[i]->is_null) { - char* emessage = NULL; - - if (!param_info->null_allowed) { - emessage = tprintf("Null not allowed for parameter %s " - "on model %s.", - param_info->name, mdfast->gen.GENmodName); - } else if (param_info->default_value_siz == 0) { - if (param_info->type == MIF_STRING) - continue; // Allow NULL - emessage = tprintf("Parameter %s on model %s has no default", - param_info->name, mdfast->gen.GENmodName); - } - - if (emessage) { - LITERR(emessage); - tfree(emessage); - gc_end(); - return; - } - } else if (param_info->is_array && param_info->has_conn_ref && + if (!mdfast->param[i]->is_null && + param_info->is_array && param_info->has_conn_ref && fast->conn[param_info->conn_ref]->size != fast->param[i]->size) { - LITERR("Array parameter size on model does not match " - "connection size"); + emessage = tprintf("Size of array parameter %s on model %s " + "does not match connection size", + param_info->name, mdfast->gen.GENmodName); + } + if (emessage) { + LITERR(emessage); + tfree(emessage); gc_end(); return; } @@ -1081,7 +1065,3 @@ copy_gc(char* in) alltokens[curtoknr++] = newtok; return newtok; } - - - - diff --git a/src/xspice/mif/mifgetmod.c b/src/xspice/mif/mifgetmod.c index b38c279dd..2a43d9986 100644 --- a/src/xspice/mif/mifgetmod.c +++ b/src/xspice/mif/mifgetmod.c @@ -56,6 +56,7 @@ NON-STANDARD FEATURES #include "ngspice/mifproto.h" #include "ngspice/mifdefs.h" +#include "ngspice/mifparse.h" #include "ngspice/mifcmdat.h" #include "ngspice/suffix.h" @@ -101,8 +102,6 @@ char *MIFgetMod( char *err2; MIFmodel *mdfast; - /* Mif_Param_Info_t *param_info;*/ - /* =========== First locate the named model in the modtab list ================= */ @@ -143,34 +142,46 @@ char *MIFgetMod( /* ============== and return an appropriate pointer to it ===================== */ /* make sure the type is valid before proceeding */ - if(modtmp->INPmodType < 0) { - /* illegal device type, so can't handle */ - *model = NULL; - return tprintf("MIF: Unknown device type for model %s\n", name); - } + if (modtmp->INPmodType < 0) { + /* illegal device type, so can't handle */ + + *model = NULL; + return tprintf("MIF: Unknown device type for model %s\n", name); + } /* check to see if this model's parameters have been processed */ - if(! modtmp->INPmodfast) { + + if (!modtmp->INPmodfast) { + struct IFdevice *device; + int num_pars; /* not already processed, so create data struct */ - error = ft_sim->newModel ( ckt, modtmp->INPmodType, - &(modtmp->INPmodfast), modtmp->INPmodName); - if(error) - return(INPerror(error)); - /* gtri modification: allocate and initialize MIF specific model struct items */ + error = ft_sim->newModel(ckt, modtmp->INPmodType, + &(modtmp->INPmodfast), + modtmp->INPmodName); + if (error) + return INPerror(error); + + /* Allocate and initialize MIF specific model struct items. */ + mdfast = (MIFmodel*) modtmp->INPmodfast; - mdfast->num_param = DEVices[modtmp->INPmodType]->DEVpublic.num_param; + mdfast->num_param = + DEVices[modtmp->INPmodType]->DEVpublic.num_param; mdfast->param = TMALLOC(Mif_Param_Data_t *, mdfast->num_param); - for(i = 0; i < mdfast->num_param; i++) { + for (i = 0; i < mdfast->num_param; i++) { mdfast->param[i] = TMALLOC(Mif_Param_Data_t, 1); mdfast->param[i]->is_null = MIF_TRUE; mdfast->param[i]->size = 0; mdfast->param[i]->element = NULL; } - /* remaining initializations will be done by MIFmParam() and MIFsetup() */ + + /* Remaining initializations will be done by + * MIFmParam() and MIFsetup(). + */ /* parameter isolation, identification, binding */ + line = modtmp->INPmodLine->line; INPgetTok(&line,&parm,1); /* throw away '.model' */ tfree(parm); @@ -179,28 +190,37 @@ char *MIFgetMod( /* throw away the modtype - we don't treat it as a parameter */ /* like SPICE does */ - INPgetTok(&line,&parm,1); /* throw away 'modtype' */ + + INPgetTok(&line, &parm, 1); /* throw away 'modtype' */ tfree(parm); - while(*line != '\0') { - INPgetTok(&line,&parm,1); - for(j=0 ; j < *(ft_sim->devices[modtmp->INPmodType]->numModelParms); j++) { - if (strcmp(parm, ft_sim->devices[modtmp->INPmodType]->modelParms[j].keyword) == 0) { - /* gtri modification: call MIFgetValue instead of INPgetValue */ + device = ft_sim->devices[modtmp->INPmodType]; + num_pars = *device->numModelParms; + while (*line != '\0') { + INPgetTok(&line, &parm, 1); + for (j = 0; j < num_pars; j++) { + if (strcmp(parm, device->modelParms[j].keyword) == 0) { err1 = NULL; - val = MIFgetValue(ckt,&line, - ft_sim->devices[modtmp->INPmodType]->modelParms[j].dataType, + val = MIFgetValue(ckt, &line, + device->modelParms[j].dataType, tab, &err1); - if(err1) { - err2 = tprintf("MIF-ERROR - model: %s - %s\n", name, err1); - return(err2); + if (err1) { + err2 = tprintf("MIF-ERROR - model: %s - %s\n", + name, err1); + return err2; } - error = ft_sim->setModelParm (ckt, - modtmp->INPmodfast, - ft_sim->devices[modtmp->INPmodType]->modelParms[j].id, - val, NULL); + + /* Store the parameter value. */ + + error = + ft_sim->setModelParm(ckt, + modtmp->INPmodfast, + device->modelParms[j].id, + val, NULL); /* free val, allocated by MIFgetValue */ - int vtype = (ft_sim->devices[modtmp->INPmodType]->modelParms[j].dataType & IF_VARTYPES); + + int vtype = + (device->modelParms[j].dataType & IF_VARTYPES); if (vtype == IF_FLAGVEC || vtype == IF_INTVEC) tfree(val->v.vec.iVec); if (vtype == IF_REALVEC) @@ -214,14 +234,16 @@ char *MIFgetMod( tfree(val->v.vec.sVec[i]); tfree(val->v.vec.sVec); } - if(error) - return(INPerror(error)); + if (error) + return INPerror(error); break; } } - /* gtri modification: processing of special parameter "level" removed */ - if(j >= *(ft_sim->devices[modtmp->INPmodType]->numModelParms)) { - char *temp = tprintf("MIF: unrecognized parameter (%s) - ignored", parm); + + if (j >= *(device->numModelParms)) { + char *temp = tprintf("MIF: unrecognized parameter " + "(%s) - ignored", + parm); err = INPerrCat(err, temp); } FREE(parm); @@ -230,20 +252,44 @@ char *MIFgetMod( modtmp->INPmodLine->error = err; + /* Make some consistency checks. */ + + for (i = 0; i < mdfast->num_param; i++) { + Mif_Param_Info_t *param_info; + char *emessage; + + param_info = device->param + i; + + if (mdfast->param[i]->is_null) { + if (!param_info->null_allowed) { + emessage = tprintf("Null not allowed for " + "parameter '%s' on model '%s'.", + param_info->name, + mdfast->gen.GENmodName); + return emessage; + } else if (param_info->default_value_siz == 0) { + if (param_info->type == MIF_STRING) + continue; // Allow NULL + emessage = tprintf("Parameter '%s' on model '%s' " + "has no default.", + param_info->name, + mdfast->gen.GENmodName); + return emessage; + } + } + } } /* end if model parameters not processed yet */ *model = modtmp; - return(NULL); - + return NULL; } /* end if name matches */ - } /* end for all models in modtab linked list */ /* didn't find model - ERROR - return NULL model */ - *model = NULL; - err = tprintf(" MIF-ERROR - unable to find definition of model %s\n", name); - return(err); + *model = NULL; + err = tprintf("MIF-ERROR - unable to find definition of model %s\n", name); + return err; } diff --git a/src/xspice/mif/mifmpara.c b/src/xspice/mif/mifmpara.c index 5f8656a9d..a6f2548a0 100644 --- a/src/xspice/mif/mifmpara.c +++ b/src/xspice/mif/mifmpara.c @@ -36,13 +36,9 @@ NON-STANDARD FEATURES ============================================================================*/ -/* #include "prefix.h" */ #include "ngspice/ngspice.h" #include -//#include "CONST.h" -//#include "util.h" #include "ngspice/ifsim.h" -//#include "resdefs.h" #include "ngspice/devdefs.h" #include "ngspice/sperror.h" @@ -53,8 +49,48 @@ NON-STANDARD FEATURES #include "ngspice/mifdefs.h" #include "ngspice/mifcmdat.h" -/* #include "suffix.h" */ +/* Check value constraints from IFS file. */ +static int check_int(int v, Mif_Param_Info_t *param_info, const char *name) +{ + if (param_info->has_lower_limit && v < param_info->lower_limit.ivalue) { + fprintf(stderr, + "Value %d below limit %d for parameter '%s' of model '%s'.\n", + v, param_info->lower_limit.ivalue, + param_info->name, name); + v = param_info->lower_limit.ivalue; + } else if (param_info->has_upper_limit && + v > param_info->upper_limit.ivalue) { + fprintf(stderr, + "Value %d exceeds limit %d for parameter '%s' of " + "model '%s'.\n", + v, param_info->upper_limit.ivalue, + param_info->name, name); + v = param_info->upper_limit.ivalue; + } + return v; +} + +static double check_double(double v, Mif_Param_Info_t *param_info, + const char *name) +{ + if (param_info->has_lower_limit && v < param_info->lower_limit.rvalue) { + fprintf(stderr, + "Value %g below limit %g for parameter '%s' of model '%s'.\n", + v, param_info->lower_limit.rvalue, + param_info->name, name); + v = param_info->lower_limit.rvalue; + } else if (param_info->has_upper_limit && + v > param_info->upper_limit.rvalue) { + fprintf(stderr, + "Value %g exceeds limit %g for parameter '%s' of " + "model '%s'.\n", + v, param_info->upper_limit.rvalue, + param_info->name, name); + v = param_info->upper_limit.rvalue; + } + return v; +} /* MIFmParam @@ -76,139 +112,152 @@ int MIFmParam( GENmodel *inModel) /* The model structure on which to set the value */ { - MIFmodel *model; - int mod_type; - int value_type; - int i; + MIFmodel *model; + Mif_Value_t *target; + Mif_Param_Info_t *param_info; + int mod_type; + int value_type; + int size, i; - Mif_Boolean_t is_array; + Mif_Boolean_t is_array; /* Arrange for access to MIF specific data in the model */ model = (MIFmodel *) inModel; + /* Check parameter index for validity */ + if ((param_index < 0) || (param_index >= model->num_param)) + return(E_BADPARM); - /* Get model type */ + /* Get model type and XSPICE parameter info. */ mod_type = model->MIFmodType; if((mod_type < 0) || (mod_type >= DEVmaxnum)) return(E_BADPARM); - - - /* Check parameter index for validity */ - if((param_index < 0) || (param_index >= model->num_param)) - return(E_BADPARM); + param_info = DEVices[mod_type]->DEVpublic.param + param_index; /* get value type to know which members of unions to access */ + value_type = DEVices[mod_type]->DEVpublic.modelParms[param_index].dataType; value_type &= IF_VARTYPES; - /* determine if the parameter is an array or not */ is_array = value_type & IF_VECTOR; - /* initialize the parameter is_null and size elements and allocate elements */ + /* Initialize the parameter is_null. */ + model->param[param_index]->is_null = MIF_FALSE; + /* element may exist already, if called from 'altermod' */ - FREE(model->param[param_index]->element); - if(is_array) { - model->param[param_index]->size = value->v.numValue; - model->param[param_index]->element = TMALLOC(Mif_Value_t, value->v.numValue); - } - else { - model->param[param_index]->size = 1; - model->param[param_index]->element = TMALLOC(Mif_Value_t, 1); + target = model->param[param_index]->element; + + if (is_array) { + size = value->v.numValue; + if (param_info->has_lower_bound && size < param_info->lower_bound) { + fprintf(stderr, + "Too few values for parameter '%s' of model '%s': %s.\n", + param_info->name, + inModel->GENmodName, + target ? "overwriting" : "fatal"); + if (!target) + return E_BADPARM; + } else if (param_info->has_upper_bound && + size > param_info->upper_bound) { + fprintf(stderr, + "Too many values for parameter '%s' of model '%s': " + "truncating.\n", + param_info->name, + inModel->GENmodName); + size = param_info->upper_bound; + } + } else { + size = 1; } + if (size > model->param[param_index]->size) { + /* Update element count and allocate. */ - /* Transfer the values from the SPICE3C1 value union to the param elements */ + model->param[param_index]->size = size; + FREE(target); + target = TMALLOC(Mif_Value_t, size); + model->param[param_index]->element = target; + } + + /* Copy the values from the SPICE3C1 value union to the param elements */ /* This is analagous to what SPICE3 does with other device types */ + if (!is_array) { + switch(value_type) { + case IF_FLAG: + target[0].bvalue = value->iValue; + break; - if(! is_array) { + case IF_INTEGER: + target[0].ivalue = check_int(value->iValue, + param_info, inModel->GENmodName); + break; - switch(value_type) { + case IF_REAL: + target[0].rvalue = check_double(value->rValue, + param_info, inModel->GENmodName); + break; - case IF_FLAG: - model->param[param_index]->element[0].bvalue = value->iValue; - model->param[param_index]->eltype = IF_FLAG; - break; + case IF_STRING: + /* we don't trust the caller to keep the string alive, so copy it */ + target[0].svalue = + TMALLOC(char, 1 + strlen(value->sValue)); + strcpy(target[0].svalue, value->sValue); + break; - case IF_INTEGER: - model->param[param_index]->element[0].ivalue = value->iValue; - model->param[param_index]->eltype = IF_INTEGER; - break; + case IF_COMPLEX: + /* we don't trust the caller to have a parallel complex structure */ + /* so copy the real and imaginary parts explicitly */ - case IF_REAL: - model->param[param_index]->element[0].rvalue = value->rValue; - model->param[param_index]->eltype = IF_REAL; - break; - - case IF_STRING: - /* we don't trust the caller to keep the string alive, so copy it */ - model->param[param_index]->element[0].svalue = - TMALLOC(char, 1 + strlen(value->sValue)); - strcpy(model->param[param_index]->element[0].svalue, value->sValue); - model->param[param_index]->eltype = IF_STRING; - break; - - case IF_COMPLEX: - /* we don't trust the caller to have a parallel complex structure */ - /* so copy the real and imaginary parts explicitly */ - model->param[param_index]->element[0].cvalue.real = value->cValue.real; - model->param[param_index]->element[0].cvalue.imag = value->cValue.imag; - model->param[param_index]->eltype = IF_COMPLEX; - break; + target[0].cvalue.real = value->cValue.real; + target[0].cvalue.imag = value->cValue.imag; + break; default: - return(E_BADPARM); - - } - } - else { /* it is an array */ - - for(i = 0; i < value->v.numValue; i++) { + return E_BADPARM; + } + } else { + for (i = 0; i < size; i++) { switch(value_type) { - case IF_FLAGVEC: - model->param[param_index]->element[i].bvalue = value->v.vec.iVec[i]; - model->param[param_index]->eltype = IF_FLAGVEC; + target[i].ivalue = value->v.vec.iVec[i]; break; case IF_INTVEC: - model->param[param_index]->element[i].ivalue = value->v.vec.iVec[i]; - model->param[param_index]->eltype = IF_INTVEC; + target[i].bvalue = check_int(value->v.vec.iVec[i], + param_info, inModel->GENmodName); break; case IF_REALVEC: - model->param[param_index]->element[i].rvalue = value->v.vec.rVec[i]; - model->param[param_index]->eltype = IF_REALVEC; + target[i].rvalue = check_double(value->v.vec.rVec[i], + param_info, + inModel->GENmodName); break; case IF_STRINGVEC: - /* we don't trust the caller to keep the string alive, so copy it */ - model->param[param_index]->element[i].svalue = - TMALLOC(char, 1 + strlen(value->v.vec.sVec[i])); - strcpy(model->param[param_index]->element[i].svalue, value->v.vec.sVec[i]); - model->param[param_index]->eltype = IF_STRINGVEC; + /* Don't trust the caller to keep the string alive, copy it */ + target[i].svalue = + TMALLOC(char, 1 + strlen(value->v.vec.sVec[i])); + strcpy(target[i].svalue, value->v.vec.sVec[i]); break; case IF_CPLXVEC: - /* we don't trust the caller to have a parallel complex structure */ - /* so copy the real and imaginary parts explicitly */ - model->param[param_index]->element[i].cvalue.real = value->v.vec.cVec[i].real; - model->param[param_index]->element[i].cvalue.imag = value->v.vec.cVec[i].imag; - model->param[param_index]->eltype = IF_CPLXVEC; + /* Don't trust the caller to have a parallel complex structure, + * so copy the real and imaginary parts explicitly. + */ + target[i].cvalue.real = value->v.vec.cVec[i].real; + target[i].cvalue.imag = value->v.vec.cVec[i].imag; break; default: - return(E_BADPARM); - - } /* end switch */ - - } /* end for number of elements of vector */ - - } /* end else */ - - return(OK); + return E_BADPARM; + } + } + } + model->param[param_index]->eltype = value_type; + return OK; }