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

Add feature to select simulator programatically #805

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

Conversation

nfrancque
Copy link

@nfrancque nfrancque commented Mar 6, 2022

Add set_simulator method exposed to user for simpler switching between simulators

#793

@nfrancque
Copy link
Author

@LarsAsplund Please review. I don't have much experience with tox but it seems natural to change the environment variable settings in the pyproject.toml for the acceptance tests to use the new interface. I ran a quick few tests locally adding set_simulator in the run.py of acceptance/artificial. Seems to work but wasn't sure what the best way would be.

@LarsAsplund
Copy link
Collaborator

LarsAsplund commented Jul 17, 2022

@nfrancque Today the selection of simulator is done in two steps based on the user's PATH and VUNIT_SIMULATOR:

  1. The simulator class is selected early when the VUnit object is created and then there are a number of other things based on that selection. https://github.com/VUnit/vunit/blob/master/vunit/ui/__init__.py#L156-L176
  2. The simulator instance is created late when main has been called (https://github.com/VUnit/vunit/blob/master/vunit/ui/__init__.py#L775)

Rearranging these events makes it harder to see if there are changes in behavior that would break backward compatibility. For that reason I suggest that the programmatic approach to changing the simulator is the same as doing step 1 and that step 2 remains where it is today. You may need to hold on to some of the arguments in the set_simulator call until the simulator instance is created in step 2.

Note that set_simulator must be called early after the VUnit object is created. At least add_library uses _simulator_output_path which is updated in step 1. Considering this constraint we should probably make the simulator selection arguments to from_argv, from_args, and __init__ to avoid mistakes.

Sets id of simulator, used for unique separate output directories for multiple versions of a class
param: id: The id to use
"""
if id is None:
Copy link
Contributor

Choose a reason for hiding this comment

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

Patterns like this can be simplified as

self.id = id or self.name

param: simulator: Name of a supported simulator
"""
supported_simulators = self.supported_simulators()
name_mapping = {simulator_class.name: simulator_class for simulator_class in self.supported_simulators()}
Copy link
Contributor

Choose a reason for hiding this comment

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

You declared supported_simulators on the previous line. I would either remove that and only use self.supported_simulators() everywhere or use that variable everywhere in here.

@GlenNicholls
Copy link
Contributor

GlenNicholls commented Nov 3, 2022

One request I have is to be able to also select the simulator from the CLI. When I'm debugging, it's often that I change the simulator from the command line using bash aliases. However, it'd be nice to have the ability to do this from the CLI instead of in the run.py file all the time.

My use-cases are:

  1. Debugging simulator bugs/issues when they pop up and other misc things that one simulator reports better than others like tracebacks
  2. Running tests that require encrypted IP be compiled. E.g. using GHDL for parallelizing all generic unit tests, then running modelsim to run tests that use e.g. PCIe core
  3. Running with e.g. modelsim+gui locally, and configuring CI to use GHDL. In that case, it's simpler to just have --sim modelsim|ghdl|rivierapro|.... I could probably add a custom CLI arg, but this seems like something VUnit should support directly

For 2., I would probably just use Python to run with two different simulators for all the tests to run back-to-back. But, I switch often enough that it'd be nice to change from the CLI as well.

@eine eine modified the milestones: v4.7.0, v4.8.0, v5.0.0 Apr 19, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants