Skip to content

Conversation

@Miguel4141
Copy link

Added AMS simulation support through the Xcelium plugin, with hard-coded additions to arguments and constraints as of now.

Related PRs / Issues

Type of change:

  • Bug fix
  • New feature
  • Other enhancement

Impact:

  • Change to core Hammer
  • Change to a Hammer plugin
  • Other

Contributor Checklist:

  • Did you set master as the base branch?
  • Did you state the type-of-change/impact?
  • Did you delete any extraneous prints/debugging code?
  • (If applicable) Did you add documentation for the feature?
  • (If applicable) Did you update the poetry.lock file if you updated the requirements in pyproject.toml?
  • (If applicable) Did you add a unit test demonstrating the PR?
  • (If applicable) Did you run this through the e2e integration tests?
  • (If applicable) Did you update the submodules in e2e/ if this feature depends on updated plugins?

@Miguel4141 Miguel4141 marked this pull request as ready for review May 22, 2024 22:44

# minimal flow configuration variables
design ?= pass
design ?= level_shifter
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this the design you tested on? If so, either add the sources to e2e/ or change this back.

Also, below in this file, please keep the clean target.

Copy link
Contributor

Choose a reason for hiding this comment

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

General comment on this file: you can put most of this info (starting at section 2, since the setup is common) in hammer/sim/xcelium/README.md and then link that file to the main Hammer documentation with a symbolic link like this: https://github.com/ucb-bar/hammer/blob/master/doc/CAD-Tools/Joules.md.

When doing so, remove references to your experimental branch (this is getting merged), the term "MXHammer" (doesn't mean anything), and references to your workspace path. If possible, delineate regular Xcelium (digital sim) setup vs. MX setup when you update this documentation.

Copy link
Contributor

Choose a reason for hiding this comment

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

Since this is in e2e, it would be OK to put BWRC-specific paths into this file (then link to it from the README document).

Copy link
Contributor

Choose a reason for hiding this comment

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

Empty?

f.close()
return True

"""def generate_amscf(self) -> bool:
Copy link
Contributor

Choose a reason for hiding this comment

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

Commented out?

"""

# Get analog models, schematics from directories specified in extralibs
extralib = self.get_setting("vlsi.technologies.extra_libraries")
Copy link
Contributor

Choose a reason for hiding this comment

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

This key doesn't exist... what are you trying to do/specify here? Do you need fields added to the normal vlsi.technology.extra_libraries key?

class ExtraLibrary(BaseModel):

f = open(runpath, "w+")

# Write Shebang + xrun clean
f.write("#!/bin/csh -f\n#\nxrun -clean \\\n")
Copy link
Contributor

Choose a reason for hiding this comment

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

Does this only work in csh? Generally we want users to use bash family terminals.

@@ -1,11 +1,18 @@
sim.xcelium:
# Tool version (e.g., "XCELIUM2103")
# Tool version (e.g., "XCELIUM2103")
Copy link
Contributor

Choose a reason for hiding this comment

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

Revert extra space

# Tool version (e.g., "XCELIUM2103")
version: "XCELIUM2103"

# Spectre version (e.g., "SPECTRE211")
Copy link
Contributor

Choose a reason for hiding this comment

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

Would note that this is only required for AMS.

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.

3 participants