There's quite a bit of existing code that could be improved without changing any functionality by:
Adding comments explaining what's going on and why it's done that way
Using clearer names and/or more consistent naming conventions
This issue is for tracking and discussing instances of this that don't warrant individual issues.
[ ] The name Env / Environment name is misleading or at least not very clear
the idea was to avoid something specific like "module", "design", etc that might be a good fit for some use cases but would be incorrect for others
my current proposal: IrGraph (but open to better suggestions)
don't rename the existing Env but rather use a new name when implementing #6, having different names will also help migration of existing code
[ ] Go over the methods names for IdVec
naming methods for IdVec is hard because it's a hybrid between a vector of values and a mapping from keys to values so just following what std does often conflicts depending on the perspective
[ ] Should Lit::lookup be called Lit::map instead?
the name lookup was derived from the initial use case, but even there it describes what the passed closure does, not what the method itself does
there are planned functional changes for this related to #7
[ ] Describe the representation used for SimModel and refer to it when creating or operating on it
[ ] Comment on how the representation of registers changes during construction at the point where compact indices are assigned
[ ] Avoid using step in all APIs (including internal ones)
it's often not clear whether it refers to a time step during simulation or unrolling, a step in an algorithm or a an entry in a structure that represents a sequence of operations
[ ] Instead of "(time) step", use "(time) frame". Established model checking terminology, used consistently by abc
[ ] For a sequence of operations, use "operation" or "op"
[ ] For a step in an algorithm use "iteration" or something more descriptive specific to what the algorithm does
[ ] Describe the use of a canonical polarity to reduce Lit equivalence to Var equivalence
[ ] Check if there's any inconsistencies with zero_values vs zero_phase in different parts of the code base
[ ] Avoid the ambiguous perspective that the current naming wrt to guarded vs. unguarded inputs has
It's not clear whether being guarded is a requirement for or a property of an input, and depending on the perspective it has the opposite meaning
I have no good idea for an alternative so far
The API around this will change with #6, no need to fix this in the current API
There's quite a bit of existing code that could be improved without changing any functionality by:
This issue is for tracking and discussing instances of this that don't warrant individual issues.
Env/ Environment name is misleading or at least not very clearIrGraph(but open to better suggestions)Envbut rather use a new name when implementing #6, having different names will also help migration of existing codeIdVecIdVecis hard because it's a hybrid between a vector of values and a mapping from keys to values so just following whatstddoes often conflicts depending on the perspectiveLit::lookupbe calledLit::mapinstead?lookupwas derived from the initial use case, but even there it describes what the passed closure does, not what the method itself doesSimModelSimModeluse a dedicated enum with helpful names instead of an OptionSimModeland refer to it when creating or operating on itstepin all APIs (including internal ones)zero_valuesvszero_phasein different parts of the code base