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

Strict parameter mode concept #346

Open
MichaelSchoenfelder-SiV opened this issue Jul 8, 2022 · 3 comments
Open

Strict parameter mode concept #346

MichaelSchoenfelder-SiV opened this issue Jul 8, 2022 · 3 comments
Labels
component: sparta Issue is related to sparta framework enhancement Enhancement or request

Comments

@MichaelSchoenfelder-SiV
Copy link
Contributor

Problem: simulators usually define a default value for parameters. These default values are almost always wrong for a mature simulator. It would be nice if sparta could complain about important parameters that aren't defined, but yet still run with defaults. (It may do this already.)

Deeper problem: "complaining" via a warning in the log is nice, but people don't often look at those logs. In production cases (running thousands of simulations), I'd like to fail soon and fail hard rather than warn.

Suggestion: implement the concept of "strict" mode or -Werror to force a sparta-based simulator to assert for parameters that aren't defined. I am not sure if we want this for all params, or to make this a property of a parameter. In other words, by default params are happy assuming a default, but one can set a property on a parameter to assert if not defined when in the "strict" mode.

@klingaard klingaard added enhancement Enhancement or request component: sparta Issue is related to sparta framework labels Jul 9, 2022
@klingaard
Copy link
Member

Little confused on this statement:

parameters that aren't defined, but yet still run with defaults

How does a parameter run with defaults if it's not defined? Not sure I follow...

On a similar topic, however, I've been bouncing around the idea of adding a default-less parameter macro that allowed the modeler to define a parameter, but provide no default value at all. The parameter has to be set via an architecture file (--arch), the standard -p <param> <value> command line option, or the configuration command line option -c <yaml>.

Naturally, the framework would die if

  1. The parameter were defined, but no default value is given
  2. The parameter were defined and a default value is given, but it's never read (just like today)

@MichaelSchoenfelder-SiV
Copy link
Contributor Author

I was sloppy in my use of the term defined. Maybe set is the correct term. Here is my example:

  1. ./sim -p foo 9 runs and is good because I want foo==9.
  2. ./sim runs and is good for testing purposes because it is annoying if the simulator can't run w/o arguments
  3. ./sim --strict -p foo 9 runs is and is good for same reason as first case
  4. ./sim --strict dies because foo is getting a default value. The default value is no good because there is no sane default for foo, or because I wrote my simulator 5 years ago and the hard-coded default is no longer valid and I don't want to change the hard-coded default

Your default-less parameter concept would meet my needs, but one couldn't run ./sim for quick checks w/o dying.

@klingaard klingaard changed the title strict mode concept Strict parameter mode concept Jul 10, 2022
@klingaard
Copy link
Member

klingaard commented Jul 10, 2022

I see. It seems we're on the same page, but with a few changes to behaviors if a default-less parameter were added and a default arch file were provided.

Assume a simulator (like in your example) has three parameters: foo, bar, and buzz.

  • foo is a default-less parameter
  • bar is a parameter with a given default value of "A"
  • buzz is a parameter with a given default value of 20

./sim has a default architecture file that sets foo and buzz only:

% cat arch/simple.yaml
top:
    my_unit.params:
        foo: 3
        buzz: 100
  1. ./sim -p foo 9 runs: foo arch value is overridden from 3 to 9, bar is default value "A", and buzz takes the arch value 100
  2. ./sim runs: foo is pulled from the architectural file 3, bar is the default value "A", and buzz takes the arch value 100
  3. ./sim --strict-parameters -p bar B runs: a non-default value for bar is given; foo and buzz are set to 3 and 100 respectively from the architecture file
  4. ./sim --strict-parameters will die as bar has a default value not being overridden in any arch file nor command line option

Thoughts?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component: sparta Issue is related to sparta framework enhancement Enhancement or request
Projects
None yet
Development

No branches or pull requests

2 participants