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

Removed test_harness_bridge and simplified harnesses #1442

Merged
merged 2 commits into from
Mar 1, 2023
Merged

Removed test_harness_bridge and simplified harnesses #1442

merged 2 commits into from
Mar 1, 2023

Conversation

nandor
Copy link
Contributor

@nandor nandor commented Feb 27, 2023

Instead, the test is now performed by fasedtests_top, in line with other tests. Also slightly simplified the test harnessed for both FASED and FireSim.

Related PRs / Issues

UI / API Impact

Verilog / AGFI Compatibility

Contributor Checklist

  • Is this PR's title suitable for inclusion in the changelog and have you added a changelog:<topic> label?
  • Did you add Scaladoc/docstring/doxygen to every public function/method?
  • Did you add at least one test demonstrating the PR?
  • Did you delete any extraneous prints/debugging code?
  • Did you state the UI / API impact?
  • Did you specify the Verilog / AGFI compatibility impact?
  • If applicable, did you regenerate and publicly share default AGFIs?
  • If applicable, did you apply the ci:fpga-deploy label?
  • If applicable, did you apply the Please Backport label?

Reviewer Checklist (only modified by reviewer)

Note: to run CI on PRs from forks, comment @Mergifyio copy main and manage the change from the new PR.

  • Is the title suitable for inclusion in the changelog and does the PR have a changelog:<topic> label?
  • Did you mark the proper release milestone?
  • Did you check whether all relevant Contributor checkboxes have been checked?

@nandor nandor added the changelog:omit Do not include this PR in the changelog label Feb 27, 2023
@nandor nandor force-pushed the init-finish branch 2 times, most recently from cac9e4a to 6c2a8b3 Compare February 28, 2023 20:22
Base automatically changed from init-finish to main February 28, 2023 23:44
@nandor nandor force-pushed the test-tops branch 2 times, most recently from 92fbc89 to 1b8e3df Compare March 1, 2023 13:54
@nandor
Removed test_harness_bridge and simplified harnesses
c1451eb 
Instead, the test is now performed by `fasedtests_top`, in line with other tests.
Also slightly simplified the test harnessed for both FASED and FireSim.
@abejgonzalez
Copy link
Contributor

Something went wrong in the workflow monitor for CI run 4304296999. Verify CI instances are terminated properly. Must be checked before submitting the PR.

Exception Message:

Expecting value: line 1 column 1 (char 0)

Traceback Message:

Traceback (most recent call last):
  File "/opt/conda/envs/firesim/lib/python3.11/site-packages/requests/models.py", line 971, in json
    return complexjson.loads(self.text, **kwargs)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/opt/conda/envs/firesim/lib/python3.11/json/__init__.py", line 346, in loads
    return _default_decoder.decode(s)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/opt/conda/envs/firesim/lib/python3.11/json/decoder.py", line 337, in decode
    obj, end = self.raw_decode(s, idx=_w(s, 0).end())
               ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/opt/conda/envs/firesim/lib/python3.11/json/decoder.py", line 355, in raw_decode
    raise JSONDecodeError("Expecting value", s, err.value) from None
json.decoder.JSONDecodeError: Expecting value: line 1 column 1 (char 0)

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "/home/centos/firesim/.github/scripts/./workflow-monitor.py", line 82, in main
    print(f"HTTP GET error: {res.json()}. Retrying.")
                             ^^^^^^^^^^
  File "/opt/conda/envs/firesim/lib/python3.11/site-packages/requests/models.py", line 975, in json
    raise RequestsJSONDecodeError(e.msg, e.doc, e.pos)
requests.exceptions.JSONDecodeError: Expecting value: line 1 column 1 (char 0)

@abejgonzalez
Copy link
Contributor

Instances for CI run 4304296999 were supposedly terminated. Verify termination manually.

davidbiancolin
davidbiancolin reviewed Mar 1, 2023
View reviewed changes
Copy link
Contributor

@davidbiancolin davidbiancolin left a comment

Choose a reason for hiding this comment

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

The review was just left pending...

sim/midas/src/main/cc/core/systematic_scheduler.h
// Adds a new task to scheduler.
void register_task(task_t &&task, uint64_t first_cycle);
void register_task(uint64_t first_cycle, task_t &&task);
Copy link
Contributor

Choose a reason for hiding this comment

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

Was there technical reason for switching the order here? If so i'm curious

Copy link
Contributor Author

Choose a reason for hiding this comment

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

With clang-format, function calls look pretty bad unless the lambda is the last argument.

sim/midas/src/main/cc/core/systematic_scheduler.h Outdated Show resolved Hide resolved
@nandor nandor enabled auto-merge (squash) March 1, 2023 21:20
@nandor nandor merged commit a81e372 into main Mar 1, 2023
@nandor nandor deleted the test-tops branch March 1, 2023 23:42
This was referenced Mar 6, 2023
Merged
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@davidbiancolin davidbiancolin davidbiancolin approved these changes

Assignees
No one assigned
Labels
changelog:omit Do not include this PR in the changelog
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@nandor @abejgonzalez @davidbiancolin