Hi all, I think I see an option which could accomplish both what Nilay and Brad want. It seems to me that the problem is that in the run script ( se.py/fs.py/etc) it is a complication to import Options.py, instantiate optparse.OptionParser, then for every file you subsequently include and for se/fs call <module>.addOptions(parser). I think a better way would be to instantiate optparse.parser in Options.py then have each module ( se.py/fs.py/Ruby.py/etc) import Options.py and add their options to the parser at module initialization. In the run scripts, after all subsystems have been imported (and automatically initialized) then get parser from Options.py and run parser.parse_args() just as it is now.
If this sounds reasonable to people I can upload a patch. Jason On Fri, Mar 30, 2012 at 3:31 PM, Beckmann, Brad <[email protected]>wrote: > Nilay, > > The basic problem here is that fs.py, se.py, and ruby_fs.py include a lot > of useful functionality that people don't want to duplicate. I don't think > asking users to maintain separate config files is the right answer. > Instead we need to make these config files more modular. Also we need to > avoid unnecessarily moving more unrelated config parameters into common > files. Jason has some ideas on how to do that and I think he'll post a > message soon on that topic. > > The goal to share more configuration code with the regression tester is > great. However, I think we need to do that by sharing more modules between > the two, not moving more information into the few modules that already are. > > Brad > > > > -----Original Message----- > > From: Nilay Vaish [mailto:[email protected]] > > Sent: Friday, March 30, 2012 9:33 AM > > To: Beckmann, Brad > > Cc: Steve Reinhardt; gem5 Developer List; Gabe Black > > Subject: RE: [gem5-dev] Review Request: Config: Change the way options > > are added > > > > Brad > > > > * The functions for adding options introduced by the patch could have > been > > placed in se.py and fs.py. I did not do that because ruby_fs.py and > fs.py have > > options in common, and to avoid duplication those options were placed in > > Options.py. And the options in se.py were also moved to Options.py so > that > > a common file has all the options in it. We can always move these > functions > > back in to their respective files. > > > > * Ruby's options are being added separately via function calls, so I did > not see > > any need to change it. I certainly feel that a lot of duplication exists > across > > scripts in configs/ruby directory, but it would require some thinking on > how > > to combine the scripts together. > > > > * These configuration files are examples which can be used to write > config > > files for a specific need. I don't know if the files in config/ change > more often > > that those in src/. But assuming that's true, any patches for these > files that > > users maintain will need to be rebased more often than other patches. I > > suggest that users should maintain separate config files instead of > modifying > > se/fs.py, so that they are not affected. > > > > * One of the things that I felt is worth spending time on, is to make > > regression tests use se/fs.py by setting options properly instead of > redoing > > the work of se/fs.py. And I think it is with this aim that I posted the > patch. > > > > -- > > Nilay > > > > On Thu, 29 Mar 2012, Beckmann, Brad wrote: > > > > > The reason I protest to this change is because I feel it is a step in > > > the wrong direction. As Steve says, the biggest pain point for us to > > > maintain patches that work on top of gem5 is the configuration scripts. > > > This changeset would centralize several system specific options into > > > Option.py which will likely cause unnecessary conflicts when future > > > parameters are added and seems to be contrast to the desire for more > > > modularity. Furthermore, the changeset uses a confusing policy where > > > most system specific parameters are put in Options.py but Ruby > > > protocol options still remain in their respective py config files. > > > Finally, there are a few minor semantic changes in this changeset that > > > just don't seem really worth it. > > > > > > Sorry to push back so much on this, but my recent experience with > > > rebasing config files have made me leery of these sorts of changes. > > > > > > Brad > > > > > > From: Steve Reinhardt [mailto:[email protected]] > > > Sent: Wednesday, March 28, 2012 5:13 PM > > > To: gem5 Developer List > > > Cc: Nilay Vaish; Beckmann, Brad; Gabe Black > > > Subject: Re: [gem5-dev] Review Request: Config: Change the way options > > > are added > > > > > > > > > On Wed, Mar 28, 2012 at 4:11 PM, Gabe Black > > <[email protected]<mailto:[email protected]>> wrote: > > > I'm assuming Steve and Ali have already weighed the pros and cons which > > is why they were ok with it going in. > > > > > > Just to weigh in on this point... I approved it because it looked > > > reasonable (i.e., not unreasonable), but I did not do an extensive > > > cost/benefit analysis. I didn't really think about the impact on > > > existing scripts. > > > > > > I will mention that config script modularity is a real problem we > > > have, and it would be nice to see steps toward making things more > > > modular (without having lots of duplicate code). I don't think this > > > change is a step in the wrong direction, but I'm not sure if it's a > > > step forward or just sideways. > > > > > > Steve > > > > > > > > > _______________________________________________ > gem5-dev mailing list > [email protected] > http://m5sim.org/mailman/listinfo/gem5-dev > _______________________________________________ gem5-dev mailing list [email protected] http://m5sim.org/mailman/listinfo/gem5-dev
