Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

on analogfunction args #68

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from
Open

on analogfunction args #68

wants to merge 1 commit into from

Conversation

felix-salfelder
Copy link
Member

perhaps required to fix #67

transport probe dependency from analogfunction args to args flagged as
'output'. this seems required for dependency tracking in cases such as

analogfunction square(out in) begin
  output out
  input in
  out=in*in
end

x = V(a)
square(x)
I(b) <+ x

with this patch, "V(a)" is a probe in the contibution "I(b) <+ x".

(something else is still missing..)
@tvrusso
Copy link

tvrusso commented Jun 19, 2017

I checked out this branch and ran it over the "rlc_AF.va" file using the "html_params.xml" generator I'd included in issue #67. It threw some warnings about "probe" being an invalid attribute, and while it does mark the two output variables with probe dependence that had previously shown no probe dependence, it doesn't set the variables' "setininstance/setinmodel/setinevaluate" attributes.

Looking over the commit casually, I see that it executes very early in function processing, before the recursive setting of argument dependencies. So while this commit is a step in the right direction, I'm not sure it's exactly right.

I briefly tried to move the new block to a later portion of the function processing of "e:dependency" (after the "choose" block that immediately follows it in the current analogfunction branch) and also tried to add a similar block to set dependency on variables the same way that "probe" dependence is propagated but I then got even more warnings about invalid attributes and a few downstream errors.

@felix-salfelder
Copy link
Member Author

thanks for trying

.. while it does mark the two output variables with probe depend

that was the only thing i knew was required.

the problem i ran into is variable declarations. in gnucap-adms, block variable declarations are ejected roughly for lhs of assignments in that block (inherited from old ngspice templates?). this is no longer sufficient for CapacitorCharge in your example.

i then noticed thar rhs/function does not list analoguefunction calls (maybe intentional?), so they are not easily accessible. if i push them in implicit.xml, adms crashes.

will need to get around that before i can possibly fix the original problem...

@tvrusso
Copy link

tvrusso commented Jun 19, 2017

Yes, Xyce, too, only emits variable declarations for the LHS of assignments (precisely because we copied the approach used in ngspice templates back when the Earth was still cooling). Once all the other issues are worked out, we'll have to adapt that approach to emit declarations for variables that only get "assigned" to in analog function "output variable" slots.

As I remarked in #67, having variable dependence propagated, too, can be important if emitting code designed for sensitivity analysis.

@tvrusso
Copy link

tvrusso commented Jun 19, 2017

Oh, and thank you for looking into this so quickly. I expected it would be difficult to fix.

@felix-salfelder
Copy link
Member Author

felix-salfelder commented Jun 19, 2017

Xyce, too, only emits variable declarations for the LHS of assignments

then, you might be interested in #69 (i dont know what the solution is).

for going further, your .xml scripts might come in handy. would you agree to include them with adms (under an appropriate license)?

@tvrusso
Copy link

tvrusso commented Jun 19, 2017

The "xyceBasicTemplates.xml" and "html_params.xml" files are part of the open-source components of Xyce, and are therefore GPL'd. I had not realized that there are no comments in there indicating this, and will make sure that in future releases there will be.

If you add this text to comments at the top of the files, then there should be no reason you can't include them with ADMS if you feel they might be useful.

Copyright Notice

Copyright 2002 Sandia Corporation. Under the terms
of Contract DE-AC04-94AL85000 with Sandia Corporation, the U.S.
Government retains certain rights in this software.

Xyce(TM) Parallel Electrical Simulator
Copyright (C) 2002-2017 Sandia Corporation

This program is free software: you can redistribute it and/or modify
it under the terms of the GNU General Public License as published by
the Free Software Foundation, either version 3 of the License, or
(at your option) any later version.

This program is distributed in the hope that it will be useful,
but WITHOUT ANY WARRANTY; without even the implied warranty of
MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
GNU General Public License for more details.

You should have received a copy of the GNU General Public License
along with this program.  If not, see &lt;http://www.gnu.org/licenses/&gt;

tvrusso added a commit to tvrusso/ADMS that referenced this pull request Aug 8, 2019
Per issue Qucs#67, the e:dependency template in adms.implicit.xml will
propagate probe and variable dependencies from analog function
arguments into the top-level expression in which the function is
called (generally the RHS of an assignment), but does NOT do the same
thing for any output arguments that might exist in the analog
function.

Some new CMC standard models (e.g. the MVSG-HV model) use analog
functions in this manner, and it would be good to fix it.

This commit augments the e:dependency template so that it completely
propagates all dependencies from function arguments into any output
variables that might exist.  It is a no-op if there are no such
function output arguments.

This commit also obsoletes pull request Qucs#68.
Unlike that PR, this one makes sure to perform dependency checking on
the input arguments before copying their probe dependencies, and
unlike that PR, it also copies their variable dependencies.  Also
unlike that code, it will correctly handle output arguments that are
not at the beginning of the argument list, because it selects all
arguments in the function definition in order and uses their position
in the global list rather than selecting only the output arguments and
using their position in the truncated list.

Issue Qucs#67
PR Qucs#68
@tvrusso
Copy link

tvrusso commented Aug 9, 2019

Xyce, too, only emits variable declarations for the LHS of assignments

then, you might be interested in #69 (i dont know what the solution is).

for going further, your .xml scripts might come in handy. would you agree to include them with adms (under an appropriate license)?

@felix-salfelder : In PR #85 I have a fix for #67, and the "xyceBasicTemplates.xml" file included in the tarball I attached to that PR for verification also has a hack that we just started using in Xyce to handle declarations of variables that are used as output variables to analog functions. How I did it might interest you for gnucap. In that file, you can look for the template "collectAssignedVariables" which originally just recursively scanned a block looking for variables that appear on the LHS of assignments. In the version attached to PR#85, it also detects analog functions with output variables and adds them to the list of "assigned" variables it's creating.

I did not find that it was necessary for analog functions to appear in the "function" list of an expression to accomplish this, but it did take some recursive template trickery.

tvrusso added a commit to tvrusso/ADMS that referenced this pull request Nov 28, 2022
Per issue Qucs#67, the e:dependency template in adms.implicit.xml will
propagate probe and variable dependencies from analog function
arguments into the top-level expression in which the function is
called (generally the RHS of an assignment), but does NOT do the same
thing for any output arguments that might exist in the analog
function.

Some new CMC standard models (e.g. the MVSG-HV model) use analog
functions in this manner, and it would be good to fix it.

This commit augments the e:dependency template so that it completely
propagates all dependencies from function arguments into any output
variables that might exist.  It is a no-op if there are no such
function output arguments.

This commit also obsoletes pull request Qucs#68.
Unlike that PR, this one makes sure to perform dependency checking on
the input arguments before copying their probe dependencies, and
unlike that PR, it also copies their variable dependencies.  Also
unlike that code, it will correctly handle output arguments that are
not at the beginning of the argument list, because it selects all
arguments in the function definition in order and uses their position
in the global list rather than selecting only the output arguments and
using their position in the truncated list.

Issue Qucs#67
PR Qucs#68
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants