Hi Malek, Can you provide a concrete example of what you are trying to do / how you are using the cluster topology? It still seems to me that adding a controllers parameter with a default value of the empty list to the Cluster __init__ will solve the problem, but maybe I don't understand what problem the patch is trying to solve.
Additionally, I don't believe that using --topology=Cluster from the command line makes sense. If all of the controllers are in a single cluster, the cluster topology is the same as a single crossbar. When I originally created the cluster topology, the idea was that users would have to manually create the cluster layout in the protocol config file. Jason On Mon, Apr 21, 2014 at 10:23 AM, Malek Musleh <[email protected]>wrote: > I think that, as you said, broke other topologies, let alone the gem5-gpu. > The original fix didn't let MeshDirCorners run. Is that not the case > anymore? > > Nilay's most recent response is still not clear to me: If we don't modify > Simulation.py > or fs/se.py or Ruby.py, then what options are there? Some parameter check > has to come from one of those files. The entire system is configured and > instantiated from those 3 files, If not one of those 3, then which one? > > Reverting to the original diff would require more changes to handle the > other topologies, but it won't be in a direction that removes > topology-specific checks, so its not clear if doing that would be useful, > or resolve the concern that was raised. > > > > > On Mon, Apr 21, 2014 at 10:06 AM, Jason Power <[email protected]> wrote: > >> This is an automatically generated e-mail. To reply, visit: >> http://reviews.gem5.org/r/2224/ >> >> On April 10th, 2014, 4:58 p.m. UTC, *Nilay Vaish* wrote: >> >> >> configs/ruby/Ruby.py<http://reviews.gem5.org/r/2224/diff/2/?file=39527#file39527line102> >> (Diff >> revision 2) >> >> def create_topology(controllers, options): >> >> 102 >> >> if options.topology == "Cluster": >> >> I am not in favour of adding such ad hoc checks to the config file. >> >> On April 15th, 2014, 3:13 a.m. UTC, *Malek Musleh* wrote: >> >> I am not quite sure what you mean. There are other instances in the config >> scripts where string comparisons are done, so there is a long precedent for >> that (including a patch that was recently committed). Do you prefer doing >> something like if type(topology) == Cluster? If so, I tried that initially, >> but it required adding additional imports to the top of Ruby.py, and created >> some other complications that prevents it from running. >> >> I think a solution is to go back to your first diff and change the line in >> Cluster.py to: >> __init__(self, controllers=[],... >> >> >> - Jason >> >> On April 10th, 2014, 4:17 p.m. UTC, Malek Musleh wrote: >> Review request for Default. >> By Malek Musleh. >> >> *Updated April 10, 2014, 4:17 p.m.* >> *Repository: * gem5 >> Description >> >> Changeset 10158:97c7a8d9ea1e >> --------------------------- >> ruby: cluster topology config fix >> >> This patch adds a missing member function, and updates the ruby config file >> to include the list of controllers for the Cluster topology. Previously the >> list of controllers was not being passed to the topology. >> >> Diffs >> >> - configs/ruby/Ruby.py (5c2ecad1a3c9) >> - configs/topologies/Cluster.py (5c2ecad1a3c9) >> - src/mem/ruby/slicc_interface/AbstractController.hh (5c2ecad1a3c9) >> >> View Diff <http://reviews.gem5.org/r/2224/diff/> >> > > _______________________________________________ gem5-dev mailing list [email protected] http://m5sim.org/mailman/listinfo/gem5-dev
