----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviews.gem5.org/r/2971/#review6992 -----------------------------------------------------------
Let me first say that this is a great initiative. It does, however, highlight some sore points about our current way of assembling, configuring and running experiments. The assembly of a graph is not easily done by if-statements and linear control code. In your case it is even two largely overlapping scripts. Also, the configuration of the options is not easily done by if-statements (how to discover what objects are there, legal combinations etc). The idea of using the objects themselves to populate some of the options is a great one. I am not sure how to deal with the other issues. Again, I do not think we should gate the addition of these scripts, I just hope we can come up with some solutions to these problems. configs/learning_gem5/part1/caches.py (line 71) <http://reviews.gem5.org/r/2971/#comment6003> This is a neat concept, and something that extends far beyond a simple education script imho. I am not discouraging the use of this idea, I just think we should not use it in the examples until that is "the way things are done". configs/learning_gem5/part1/two_level.py (line 1) <http://reviews.gem5.org/r/2971/#comment6005> There is a rather unfortunate lack of re-use between the two scripts. I realise that they are intended to be clear and description (not short), but it irks me that they are so verbose. Could we use something like the base_config in the test.configs? configs/learning_gem5/part1/two_level.py (line 72) <http://reviews.gem5.org/r/2971/#comment6004> Odd indentation - Andreas Hansson On July 16, 2015, 2:12 p.m., Jason Power wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://reviews.gem5.org/r/2971/ > ----------------------------------------------------------- > > (Updated July 16, 2015, 2:12 p.m.) > > > Review request for Default. > > > Repository: gem5 > > > Description > ------- > > Changeset 10923:8f90545cd9fe > --------------------------- > config: Add configs scripts used in Learning gem5 > > Added a new directory in configs (learning_gem5) to hold the scripts that are > used in the book. See http://lowepower.com/jason/learning_gem5/ for a working > copy. For now, only the scripts in Part 1: Getting started with gem5 > have been added. A separate patch adds tests for these scripts. > > > Diffs > ----- > > configs/learning_gem5/part1/two_level.py PRE-CREATION > configs/learning_gem5/part1/simple.py PRE-CREATION > configs/common/SimpleOpts.py PRE-CREATION > configs/learning_gem5/README PRE-CREATION > configs/learning_gem5/part1/caches.py PRE-CREATION > > Diff: http://reviews.gem5.org/r/2971/diff/ > > > Testing > ------- > > > Thanks, > > Jason Power > > _______________________________________________ gem5-dev mailing list [email protected] http://m5sim.org/mailman/listinfo/gem5-dev
