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

Implementation of routing algorithm, fix some bug and add restriction for config #90

Merged
merged 13 commits into from
Mar 8, 2021
Merged

Implementation of routing algorithm, fix some bug and add restriction for config #90

merged 13 commits into from
Mar 8, 2021

Conversation

yangyt96
Copy link
Contributor

@yangyt96 yangyt96 commented Feb 4, 2021 •
edited
Loading 

- This implementation of routing algorithm XYZ is compatible with older version (theoretically if there is no bug :D). 
- This is an extension routing algorithm XYZ for 2D-Torus and Ring
- Tested for 2D-Torus urand 100000ns: 4x4, 4x8, 8x8, 8x16, 16x16; all pass
- Tested for Ring urand 100000ns up to 64 nodes; all pass

- Add 3D vector tools - absolute value (38afed6)

- The segmentation fault is fixed (dd57894), which is due to the max value of bufferReportRouter equal to the amount of router 
- Add some restriction for config to prevent some other bugs (588c6c8)

TODO:
- implementation for 3D torus and its routing algorithm
- more updates for routing algorithm of torus

@yangyt96
[fix] segmentation fault for the value of bufferReportRouter
dd57894 
	- The count of buffer is from 0 to (#router-1), so it is not '<=' and should be '<', otherwise segmentation fault occurs
@yangyt96
[chore] add restriction for bufferReportRouters
588c6c8
@yangyt96
[feat] add absolute function for 3D vector
38afed6
@yangyt96
[chore] make assertion to make sure each connection is between 2 nodes
3705ad7
@yangyt96
[feat] routing algorithm XYZ for 2D-torus and ring
1af0145 
	- This implementation is compatible with older version (theoretically), however it will run only for XYZ routing algorithm.
	- Tested for 2D-torus urand 100000ns: 4x4, 4x8, 8x8, 8x16, 16x16; all pass
	- Tested for ring urand 100000ns up to 64 nodes; all pass

TODO:
	- implementation for 3D torus and its routing algorithm
	- more updates for routing algorithm of torus
@yangyt96
[chore] change the interpretation of the code
75c003b
jmjos
jmjos requested changes Feb 8, 2021
View reviewed changes
Copy link
Owner

@jmjos jmjos left a comment

Choose a reason for hiding this comment

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

Hi! Looks good in general, except two things. Please do the following:

  1. Please do not modify the bin/...py files directly. Rather, make a new package as discussed offline. I would like to keep the bin folder constant for now for legacy reasons.
  2. Please add urand tests to the CI for the new topologies and routings. (see .travis.yml)
  3. Please use the new packaged-python classes for the tests as much as possible.
    Thanks!

@yangyt96
Copy link
Contributor Author

I revert the commit 588c6c8, but this has removed the feature for adding "topology" into config.xml, so the simulator cannot use the new feature of ring and torus routing. Do I still need to add this topology feature (in xml_writers.py) into too? (Checkout d9bae63 bin/xml_writers.py, line 598, 600).

@jmjos
Copy link
Owner

jmjos commented Feb 12, 2021

Yes, please add it. But you can add it to the "new" xml_writers.py in the ratatoskr-tools package only. That would be fine to foster migration to the new interface!

@yangyt96
[fix] routing error for 2D-Torus when x=2 or y=2
b6ada92 
	- Due to the problem when x=2, we need to know what is the direction of target node to the source node (west or east). Here, we still define it same as mesh.
	- Also fix for y-axis too.
@yangyt96
[rmv] unnecessary code for z-axis
9a13fbe
@yangyt96
[style] style formatting for better readability
d7a2a56
@yangyt96
[chore] add output directory
152d8ff 
	- if not given --outputDir, the default is current working directory
@yangyt96
[chore, rmv] restructure the input options of the simulator
6c5d69f 
	- shell input with 2 arguments is removed (this might cause incompatibality to the older version, if required will be added again, however this kind of input method is not secure)
	  e.g. $./sim config/config.xml
	- added new input options:
		--configPath
		--networkPath
@yangyt96
[rmv] remove unnecessary line
5ef37e4
@yangyt96
Copy link
Contributor Author

Can we make a version number after this pull request is merge? So for the ratatoskr-tools documentation can mention that the minimum version requirement for the simulator, this would be more friendly for the user :D

@yangyt96
Copy link
Contributor Author

yangyt96 commented Mar 1, 2021

Hi! Looks good in general, except two things. Please do the following:

  1. Please do not modify the bin/...py files directly. Rather, make a new package as discussed offline. I would like to keep the bin folder constant for now for legacy reasons.
  2. Please add urand tests to the CI for the new topologies and routings. (see .travis.yml)
  3. Please use the new packaged-python classes for the tests as much as possible.
    Thanks!

It is done with this package :D #89 #92
https://github.com/jmjos/ratatoskr-tools/tree/dev-yang

@jmjos jmjos merged commit 164ea6c into jmjos:new-features-2021-01 Mar 8, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jmjos jmjos jmjos approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@yangyt96 @jmjos