Options in the Python API

[maliberty]

The python api differs from tcl in being more OO (objects/methods) rather than command oriented. I'd like to discuss standardizing how that API accept options. Current APIs are variable:

• Some use state building commands. For example pdn has define_pdn_grid, add_pdn_stripe, etc.
• Some persistently hold an options object that can be modified by the user. For example TritonCTS::getParms()
• Some take their options as arguments. For example Resizer::repairHold(...)

Complicating cases are:

• Tools that call other tools. For example gpl calls grt and rsz
• Incremental operations that retain state between invocations. For example grt and sta

My best thought on how to standardize is to have a persistent set of default options and the ability to pass an overriding set. A sketch of the idea:

class ToolOptions
{
public:
  ToolOptions(); // a well-defined default state
  void setOptionA(int value);
  void setOptionB(bool value);
};

class Tool
{
public:
  static ToolOptions* getDefaultOptions();

  // if options is nullptr then the default options are used.
  void task(ToolOptions* options = nullptr);

private:
  static ToolOptions* default_options_;
};

One use model would be pre-configuration (in C++ but similarly in python)

// setup
auto default_options = Tool::getDefaultOptions();
default_options.setOptionB(false);

// later
tool->task();

Another use model could be

ToolOptions options;
options.setOptionB(false);
tool->task(options);

This can be extended to the case of one tool calling another internally with an API like:

void task(ToolOptions* options = nullptr,
          AnotherToolOptions* options = nullptr);

For a sufficiently complicated tool the options object could itself become a sub-configuration builder. For example:

class PdnOptions
{
public:
  RingOptions* add_ring();
};

The case of an incremental tool is tricky. If asked to do an update I think it should do so according to the last set of options (default or not) that it used. This implies that it must keep a copy of any non-default options object.

I'm open to other ideas or improvements. I'd love to hear from folks @gadfort @macd @QuantamHD @cdleary @osamahammad21 @antonblanchard @donn @rovinski

[maliberty]

@proppy

[QuantamHD]

I like the idea, but I think I would make a modification to a builder architecture, which I think makes sense for two reasons.

1. It allows you to do more validation in your builder which would give us the ability ensure that a user can never create an invalid tool configuration. For example creating a PDN that has straps repeat at a greater pitch than the width of the chip (Which I've run into)
2. I like the idea of ToolOptions being an Immutable object. It means that a tool cannot modify the options which I think reduces the possibility of tools doing weird things with user provided options.

class ToolOptionsBuilder : OptionBuilder {
public:
  /*
  * Not necessarily required in all cases, but could help build default options for a particular PDK, or
  * core size etc.
  */
  ToolOptionsBuilder(Design *);

  void setOptionA(int A);
  void setOptionB(bool B);
  ErrorOr<ToolOptions> create(); // could also throw an exception.
}

class ToolOptions : ToolOption{
private:
   ToolOptions(int A, bool B); // a well-defined default state

public:
   int getOptionA();
   bool getOptionB();
};
 
class Tool {
public:
   static ToolOptions* getDefaultOptions(); // calls ToolOptionsBuilder(...).create(); 
 
   // if options is nullptr then the default options are used.
   void task(ToolOptions* options = nullptr); 
};

// setup
auto default_options = Tool::getDefaultOptions();
default_options.setOptionB(false);

// later
tool->task();

I don't like the idea of modifying the default options object, seems very error prone.

Another use model could be

ToolOptionsBuilder options;
options.setOptionB(false);
tool->task(options.create());

I like this model, but with the builder pattern.

This can be extended to the case of one tool calling another internally with an API like:

void task(ToolOptions* options = nullptr,
          AnotherToolOptions* options = nullptr);

I think this is a greate idea too.

For a sufficiently complicated tool the options object could itself become a sub-configuration builder. For example:

class PdnOptions
{
public:
  RingOptions* getRing();
};

I like this but again should be constructed using builders.

The case of an incremental tool is tricky. If asked to do an update I think it should do so according to the last set of options (default or not) that it used. This implies that it must keep a copy of any non-default options object.

I think there's also another problem

1. The common use mode of the tools is to do one step then dump an ODB file. I don't think we can get rid of this use mode, because then it would be impossible to run the tool incrementally. Which is making me feel like we need to serialize them into the database.

I've thought about it a bunch, and I think storing a hidden only history from a user is probably not the right API instead I suggest.

class Tool {
public:
   static ToolOptions* getDefaultOptions(); // calls ToolOptionsBuilder(...).create();
   set incrementalOptions(ToolOptions* options);
 
   // if options is nullptr then the default options are used.
   void task(ToolOptions* options = nullptr);
 
private:
  ToolOptions* incremental_options_;

};

On construction the incrementalOptions are set to what's provided in the constructor, or can be updated by the user.

This brings up another question about incremental APIs. @maliberty how do you envision a tool getting access to a Tool object will the user need to pass one into the options, or something else?

[QuantamHD]

In that case the validation must go in create()

[rovinski]

Sorry - deleted the question because I realized the answer as soon as I posted :)

So the normal work flow would look like

1. Declare object
2. Set options
3. object::create()
4. Use created object in tool

Correct?
For C++ I suppose that's fine, but I'm trying to visualize how this would boil up to Tcl / Python.

[QuantamHD]

The object and the builder are two separate classes. The object hides its constructor from the user, and the only way you can get an instance is through the builders create routine

Which means invalid objects can't be created.

[QuantamHD]

See https://en.m.wikipedia.org/wiki/Builder_pattern

[maliberty]

You get the Tool from the Design object: https://github.com/The-OpenROAD-Project/OpenROAD/blob/master/include/ord/Design.h

[rovinski]

It's a very interesting question and I feel like I've become jaded because of how inconsistent commercial tools are. It seems like tools (both commercial and OpenROAD) always consist of some combination of the three options you mentioned. What's worse is that commercial tools can be some kind of weird blend of global variables, app variables, tool variables, etc. all with different methods for accessing them... it can get overwhelming.

I think for the most part, I like the idea of doing away with arguments to commands and having all options passed through some kind of options object. The caveat is that I am split on whether that's appropriate for very simple commands that only take one parameter, like read_odb [filename].

Whatever the decision is, I definitely want to see a common command / interface that lets a user easily query all parameters that can be set for a tool. For example:

openroad> list_parameters Tool
Parameters for command 'Tool':
Parameter  Type  Value  Default Description
-------------------------------------------
A          int   1      1       Short description of A
B          bool  1      0       Short description of B
-------------------------------------------
Use 'help Tool' for more information on this command's parameters

openroad> help Tool
Parameter  Definition
------------------------
A          This is a long-form description of A. Here are the valid values, maybe some tips, etc.
           The description could be multiple lines.

B          This is a long-form description of B. Here are the valid values, maybe some tips, etc.

Use 'list_parameters Tool' to list the current values for this command's parameters

Hidden state / parameters is one of the things I hate the most about commercial tools. At the same time, a user definitely shouldn't need to set parameters they don't know about, like bin grid size for GPL.

These comments are mostly concerning the user-side interface. As for the developer / C++ API, I don't think I have strong opinions. I think the proposed interface sounds ok.

[maliberty]

I had a similar thought about cases with just a few arguments. Its a bit annoying to create an object just to pass one mandatory argument like in read_db. I was thinking of a rule like <4 args can be passed directly. If so that would be another argument for validation to be in the tool.

Completely agree on list & help

[QuantamHD]

I think in general we should do things "that make sense"™. If something is clear and non-error prone there's no reason we shouldn't have Design* Design::read_db(std::string).

I think we should philosophically separate tcl commands from the C++/Python API. We can implement tcl commands with the C++ API that follow the current command like convention.

I think of the TCL "API" as being more of a GUI than a proper stable API. The TCL API should be implemented by the C++ API.

[rovinski]

In my mind, I foresee both Tcl and Python being thin wrappers on top of the C++ API. I think right now there is a lot of supporting code which is being duplicated between the Tcl interface and Python interface, which is bad because it gives many more opportunities for bugs / divergence between the two. If for whatever reason we want to add another SWIG-supported frontend, it would also simplify things.

I don't really want to see different programming models / APIs between the two languages. We already have an issue where old-school chip designers are very familiar with Tcl, and software people are more familiar with Python. I worry about some kind of "silo-ing" effect if the APIs aren't near-identical.

[maliberty]

I envision dropping tcl eventually. I don't see it becoming an OO api. We do need to move the validation into c++.

[rovinski]

Hmmm as much as I would be interested in moving away from Tcl, it seems like it would create friction for those used to Tcl-based tools?

I am really hoping that there can be a minimal amount of effort to maintain the frontend API on top of the SWIG interface. I think KLayout does a good job of this - the Ruby and Python APIs are nearly identical, work essentially the same, and the documentation between the two is basically shared.

[maliberty]

I'm lukewarm on a builder. Your two arguments for it were:

"It allows you to do more validation in your builder". You can just as easily do this validation in the task. If you want to do it in the builder then you will have to restrict when it can be used (ie not before the db is populated) which seems awkward. The builder itself will have to be constructed by the tool to give it enough state for validation.

"I like the idea of ToolOptions being an Immutable object. It means that a tool cannot modify the options which I think reduces the possibility of tools doing weird things with user provided options." I see two problems. The tool can always clone the options object and do "weird things". In gpl for example we set options before calling grt (see

OpenROAD/src/gpl/src/routeBase.cpp

Line 374 in b632f9f

// these two options must be on

). I wouldn't want to force the user to know to do so.

[QuantamHD]

I'm lukewarm on a builder. Your two arguments for it were:

"It allows you to do more validation in your builder". You can just as easily do this validation in the task. If you want to do it in the builder then you will have to restrict when it can be used (ie not before the db is populated) which seems awkward. The builder itself will have to be constructed by the tool to give it enough state for validation.

I think it comes down to "do as much validation as you can in the builder". It's obviously not going to catch everything, but we can do some validation across settings with a builder, and that's a net positive. Many builder have an isValid function which I could see being helpful for things like PDN from an interactive learning perspective.

Allowing users to construct arbitrary objects is about the same it just doesn't let us validate an option that dependent on another field. I think we want to reserve that functionality.

"I like the idea of ToolOptions being an Immutable object. It means that a tool cannot modify the options which I think reduces the possibility of tools doing weird things with user provided options." I see two problems. The tool can always clone the options object and do "weird things". In gpl for example we set options before calling grt (see

OpenROAD/src/gpl/src/routeBase.cpp

Line 374 in b632f9f

// these two options must be on

). I wouldn't want to force the user to know to do so.

Even in that case at least the users configuration remain unchanged. Also that's another argument for thr
E builder.

We could potentially move those configuration options to hidden fields that get populated by the builders create method. Where the immutableness of the objects makes sure that those fields don't go out of sync if they depend on the options picked.

If we didn't go that route grt making a copy of the options is desirable. That way we know two invocations with the user's pointer always yields the same input in.

[maliberty]

Having an isValid method that only partially checks is pretty confusing.

The tool can just take the user option as a const object if you want to prevent it being modified. It doesn't require a builder to achieve that.

[maliberty]

re "I don't like the idea of modifying the default options object, seems very error prone."

I have a somewhat similar feeling but I'm also afraid of producing very complex API calls (eg gpl). I think some people would prefer to setup the tools upfront. Also interactively it will save a lot of typing. However I am fine to remove it if there is a consensus that it isn't useful.

[maliberty]

Putting the options in the db will open it up to tons of schema updates which is undesirable. Or we would have to make the storage extremely generic which will push the validation on load problem to each tool which isn't much better. Its also error prone for the user.

I lean toward saying that on reload you don't get any incremental operations. We are living with that today, sta is not incremental across restart.

[QuantamHD]

I'm fine with that if that's what users expect, but I think as I suggested we could go with my approach of having an incremental configuration that the user could set upon reloading

[maliberty]

If you set the configuration to something other than what it was when the db was closed then you will get incorrect incremental results. That seems very error prone. If we allow incremental on reload then we should store the settings and reset if the current settings are not equal to the saved ones. Saving them brings up my other points about the schema that are not addressed.

[QuantamHD]

We could write the configuration options in proto, boost or captain proto. That way we could just dump them into the database as binary.

[gadfort]

I don't know if storing the DB is great, unless we can figure out a method that doesn't change the schema numbers constantly.
I personally think it's okay to have the user re-set those values on reloading of OpenROAD. That also avoids hidden bugs because the user set it before and forgot (@rovinski pointed that out).

[maliberty]

Serialization isn't the hard part, it is validation. Proto doesn't solve the validation issue.

[donn]

Sorry it's taken me a minute to respond to this- perhaps I can offer a bit of a different perspective being more on the user side, but:

• Lukewarm on the builder, too. I don't see the utility outside of a niche case where I may elect to build and serialize the options in OpenLane then use it in OpenROAD, but I'm fairly sure that won't be possible regardless.
• I like the idea of immutability vs. modifying a global variable. I get that means more typing, but I think it's worth it just to have less surprises. i.e. I wholeheartedly endorse this model:

ToolOptions options;
options.setOptionB(false);
tool->task(options);

From where I stand, the above model would let me write a single common file to convert OpenLane variables to an OpenROAD ToolOptions object generator, which can then be passed onto other tools.

[donn]

Also, +1 for dropping Tcl when the time comes.

[maliberty]

If people are ok with requiring explicity APIs that's my preference too. I was more concerned that people would find it more cumbersome to use (especially interactively).

[donn]

well, you know i don't use interactive mode that much so my concerns are definitely stability over user friendliness, so there's that caveat

[QuantamHD]

The goal of the builder is so that we can separate validation from tool execution in an API friendly way.

[maliberty]

@rovinski tcl is a completely different API with lots of manual coding. Converting it look like python, if possible, would introduce the same friction of it looking nothing like usual EDA tools' tcl interface. I don't think it is worth the effort to maintain two sets of APIs. Klayout supporting both python & ruby is much simpler since they are both OO languages. We could do that too more easily (though I have no desire to).

[rovinski]

Eh, I don't know 🤷‍♂️. I would just prepare for a rough transition period.

There is a certain commercial tool I'm thinking of which has OO database access commands in Tcl. odb Tcl commands are also rather OO right now. I don't think OO Tcl is too much of a leap, or at least something close.

What is the timeline and plan of action for updating the C++ API? Would you drop Tcl support after the C++ API is updated, or would you look to update Tcl support for the new C++ API while keeping the user-facing Tcl API the same?

[maliberty]

I would expect it to be a long transition. We would need to keep tcl going through the API changes and likely for a while afterwards. Even if we do keep an OO tcl API it would be a completely new interface.

I know the OO db access exists in tcl but that is about the only OO api. Everything else is not OO.

[maliberty]

Based on the discussions I'm wondering if it is preferrable to eliminate the global state and use only explicit options with a default arg:

class Tool
{
public:
  void task(const ToolOptions& options = ToolOptions());
  void complex_task(const ToolOptions& options = ToolOptions(),
                    const AnotherToolOptions& other_options = AnotherToolOptions());
};

[macd]

Ok, sounds good. The per tool default options is the way to go. These options would, of course, depend on the PDK and we can imagine other dependencies for the defaults as well. This kind of backs into (a little bit) design flow management. (ie Company or design specific options). So would there be a persistent form these defaults? I'd vote for a text based form, maybe json, rather than one of the binary forms mentioned previously. I also agree with not trying to jam these into the odb, but maybe we'd want to consider putting in the hash of the options into the odb if only to say, "yup, these two go together".

[QuantamHD]

I think the persistent form of these options would be a source code file that you wrote and import in your language of choice. I think the tools should move in the direction of the "user's chip is a program, and everything else is a library (PDK, DFFRAM, OpenRAM, Adders, Macros, Config, etc.)".

Ruby on Rails, but instead of a web server the user written program generates your chip.

[macd]

This would make it a chore for a Python user to share defaults with a tcl user, I think. My (concern|goal) is in the areas of managing a design team and making reproducible builds possible. We are not going to solve that here, but we can make it more difficult to do so. I agree with the goal of the "user's chip is a program" but everything else as a library ignores things like Makefiles and other scaffolding that goes into making a build product. So I still think a persistent text based format (that can be under revision control) is the way to go here. My choice has changed for I think toml would be better than json. Easy to read in tcl, Python, and C++.

[maliberty]

@macd wouldn't the persistent form be something the flow scaffolding should handle? The flow developer could choose the right form of persistence for their needs. For many that might just be python scripts. For others they could have a loader from whatever format they like. I'm not sure it makes sense to put that decision in OR.

[maliberty]

For example SiC uses python, eg https://github.com/siliconcompiler/siliconcompiler/blob/main/examples/aes/aes.py

[donn]

Should probably add- syntactic sugar in the form of

auto options = ToolOptions {
    {"a", "value1"},
    {"b", "value2"}
};

i.e., one that basically takes a dict, with the appropriate bridging to python:

options = ToolOptions(
   a="value1",
   b="value2",
)

would be pretty cool

[maliberty]

This gives up type checking so I wouldn't do it in c++ but could in python. Perhaps the swig layer could handle the conversion from a python dict.

[proppy]

https://docs.python.org/3/library/dataclasses.html is a good way to generate those in Python along side type annotation, I'd be great to have those layered on top of the swig generated stuff.

[maliberty]

@proppy dataclasses seems to be intended for classes where the fields are public. I don't think we should make the field public as that makes evolution harder. Is there a better solution for encapsulation?

[maliberty]

For a next step I want to make a concrete example. Looking at gpl it fits this model well is it is a single task (placement) with many options.

When I look at ifp it is more problematic. It is a tool with multiple semi-related tasks: initFloorplan, insertTieCells, and makeTracks. I don't feel it makes sense to have a single options object to cover such diverse tasks. Perhaps this would be better formulated as TaskOptions rather than ToolOptions. A TaskOptions could replace the need for overloaded methods.

This does run the risk of a proliferation of classes - worst case is one for every method. I am hopeful that the number of tasks is fairly limited per tool today.

[rovinski]

Maybe the TaskOptions could use a code generator?

[maliberty]

I created #2809 as a example of what the revised API for gpl could look like. The user tcl layer is unchanged but python and c++ see a new options object. Replace has way too many options so it produces a very large object interface.

Having made the change I'm a bit on the fence about keeping it like this versus making either (1) a code generator from a description file or (2) a more weakly typed interface using string (or enum) keys with a variant value and a single get/set method. I'm glad to hear opinions.

[maliberty]

Note that I didn't do grt and a second option arg but that is eventually needed.

[rovinski]

I wonder if it would be possible to create a parser which parses the tool class and generates an options class from that? It would be using the source file as the 'description file' to reduce duplicate code.

[QuantamHD]

I think if we're looking for generators i would stick to something that already exists. Proto, captain proto

I'd also encourage us to just eat the cost because it's simpler to work on.

[rovinski]

If there's already a system for that, sure.

[maliberty]

I'm not totally sold on either as they have limited ability to specify constraints on values (ie this is a percentage, or must be >0). If I going to use code generation then I also want to generate the validation if possible.

[maliberty]

Any thoughts on either the existing approach or my other suggestion?

[maliberty]

I came across https://abseil.io/tips/173 which gives some nice ideas for using designated initializers