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.

Sign up for GitHub

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

Jump to bottom

Mode enum #407

Open
wants to merge 9 commits into
base: master
Choose a base branch
from
Open

Mode enum #407

Changes from 3 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
0fbcfa5
Replace blank power straps mode with empty
Jingyi99 May 21, 2019
cdad2aa
Merge branch 'master' of https://github.com/ucb-bar/hammer
Jingyi99 May 22, 2019
46b4868
add Mode Enum class #400
Jingyi99 May 22, 2019
bbf024e
include 4 mode types, change string comparisons to Enum comparisons
Jingyi99 May 22, 2019
10d94d0
fixed typo
Jingyi99 May 22, 2019
ba8f207
Update src/hammer-vlsi/hammer_vlsi/hammer_tool.py
Jingyi99 May 22, 2019
c2d56e5
Update src/hammer-vlsi/hammer_vlsi/hammer_tool.py
Jingyi99 May 22, 2019
a82a9b6
fix type hinting comments
Jingyi99 May 22, 2019
397deb7
add hammer tool class method get_mode_setting
Jingyi99 May 24, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 34 additions & 0 deletions src/hammer-vlsi/hammer_vlsi/hammer_tool.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,9 @@
from .submit_command import HammerSubmitCommand
from .units import TemperatureValue, TimeValue, VoltageValue

from enum import Enum
from hammer_utils import reverse_dict

__all__ = ['HammerTool']


Expand Down Expand Up @@ -1128,3 +1131,34 @@ def verbose_tcl_append(cmd: str, output_buffer: List[str]) -> None:
"""
output_buffer.append("""puts "{0}" """.format(cmd.replace('"', '\"')))
output_buffer.append(cmd)
class ModeType(Enum):
Jingyi99 marked this conversation as resolved.
Show resolved Hide resolved
Auto = 1
Empty = 2
Manual = 3
Generate = 4
Generated = 5
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As far as I'm aware, the modes we use are:

    Auto = 1
    Empty = 2
    Manual = 3
    Generated = 4

append, prepend are part of another part of Hammer and not part of this mode enum.

Also, if we are going to create this enum, I think it may be good to also use them in some of the settings.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should change power straps to generated from generate if we're going to use generate. I personally like generate better, but generated does keep all of the modes as adjectives, which also makes sense.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, maybe we can do that as a separate issue then?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can just tack it on to #397 .

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi Edward, thanks for your comment! I checked defaults.yml and it seems that pin place mode uses generated, and power straps mode uses generate. Just to make sure, they are the same and we'll stick with generate, right?
Also, I'm not sure what it means to use the enum in the settings, could you explain a bit more?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi Jingyi- let's use generated instead of generate. We'll change the power straps mode to generate.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, so John's comment above is alluding to that problem. I think the desired action is to use generated and just ignore the power straps mode for now.

As for use, if you look in the code, right now it uses strings - the intention is to use this enum instead

hammer/src/hammer-vlsi/hammer_vlsi/hammer_tool.py

Line 896 in 8bcbc40

if bumps_mode == "empty":

Append = 6
Prepend = 7

@classmethod
def __mapping(cls) -> Dict[str, "ModeType"]:
return {
"auto": ModeType.Auto,
"empty": ModeType.Empty,
"manual": ModeType.Manual,
"generate": ModeType.Generated,
"generated": ModeType.Generate,
"append": ModeType.Append,
"prepend": ModeType.Prepend
}

@staticmethod
def from_str(input_str: str) -> "ModeType":
try:
return ModeType.__mapping()[input_str]
except KeyError:
raise ValueError("Invalid mode type: " + str(input_str))

def __str__(self) -> str:
return reverse_dict(ModeType.__mapping())[self]