Thursday, 2021-09-23

kylellkcl, did you want me to work on expected alu cases or is something else required?13:41
lkclkylel, as a "default background" thing, yes, test_caller_shift_rot.py13:44
lkclwhat would be really good would be to convert test_caller_shift_rot.py to be in the same format as test/alu/alu_cases.py13:45
lkclnow that they both support "ExpectedState", this should be fairly easy13:45
kylelok, will take a look.  Want a new bug for this?13:46
lkclthat's another "step along the way"13:46
* lkcl thinks13:46
lkclyyeah the existing one is getting pretty big (comment-wise)13:46
lkcland i'd say it's a good "stake in the ground" for preliminary investigation13:46
kylellink it to 686?13:50
lkclyes13:51
lkclotherwise it becomes impossible to find related things13:51
lkclalways important to link to at least... something13:52
lkclthen, next time you look at it, there's an extra related bugreport 2 degrees or 3 degrees away that turns out to be immediately relevant13:52
lkclbut if isolated it's virtually impossible to find anything13:52
lkclkylel, you can see i added one, here: https://git.libre-soc.org/?p=openpower-isa.git;a=commitdiff;h=907570fdb16fa0baf737a79017b28ce4b980a92613:57
lkclbut basically, what i suggest is:13:58
lkcl* get test_caller_shift_rot.py to derive from TestAccumulatorBase13:58
lkcl* copy how things are done in test_issuer.py (suite.addTest)13:59
lkcl* create your own TestRunner which calls run_tst_program14:00
lkclthe way that TestAccumulatorBase works is:14:00
lkclit accumulates a list of tests (into test_data)14:00
lkclthen you can set up **ONE** Simulator (and one PowerDecoder2, which is very expensive / timeconsuming)14:00
lkcland in a for-loop run *ALL* the instances accumulated in test_data14:01
lkclbehind a self.subTest(test.name)14:01
lkclthe function which does that needs to be named "run_all()14:01
lkcl"14:01
lkclhttps://git.libre-soc.org/?p=soc.git;a=blob;f=src/soc/simple/test/test_runner.py;h=045e96813767e2be0e0920934bc0f500f92b54e1;hb=f7b708268bf758a2cea294fd7f070be4d78acc6b#l25614:02
lkclactually...14:02
lkclhilariously...14:02
lkclyou could probably take test_runner.py as-is and literally cut out the call to run_hdl_state()!14:03
lkclcut out TestIssuerInternal at line 29214:03
lkcletc. etc.14:03
lkclif you wanted to make it a bit "cleverer", make it *optional* - in TestRunner itself - to activate TestIssuer14:04
lkclwhich might be simpler14:04
kylelguess we'll see :)14:10
lkclactually, that should be a brain-dead-simple way to do it14:19
lkclbecause effectively it's duplicated code, if you compare TestRunner.run_all to the run_tst function14:20
* lkcl hmmms and deliberates, a candle instead of a lightbulb above my head...19:31
lkcli'm thinking of a new API, kylel, one which has a "Setup" function, "Runner" function, "Prepare for Test" function, and so on19:33
lkclerr exactly like how python unit tests work, in fact19:33
lkclhttps://docs.python.org/3/library/unittest.html19:33
lkclimport unittest19:34
lkclclass WidgetTestCase(unittest.TestCase):19:34
lkcl    def setUp(self):19:34
lkcl        self.widget = Widget('The widget')19:34
programmerjakei'd recommend using the unittest api, rather than rewriting it19:36
lkclyyeah i thought that, then i got concerned about how it catches exceptions and has all the stuff about "if it starts with test_ then run it" and so on19:37
lkcland, there's no concept "step-to-next-simulation-state"19:37
lkclprogrammerjake, you saw about the (severe) impact that removing me from the "donated" list has?19:38
lkclthat's an actual public declaration, "this money was NOT paid to lkcl"19:39
lkclwhich is extremely serious.19:39
lkcli run budget-sync by looking at the output, each and every time.19:39
lkcli never ignore warnings.19:39
programmerjakewrite a class that inherits from the unittest api, you can add methods for stepping simulator state19:40
programmerjakelkcl: the change for budget-sync has the donations still going to you (it's just another user named "Luke Kenneth Casson Leighton (donated)"19:42
programmerjakeit should still be obvious that it's still lkcl19:42
programmerjakeyou ignored errors by changing the code to instead print NOT AN ERROR...19:44
programmerjakei originally put the error detection in there because i assumed that the error wouldn't be triggered if the bugzilla was consistent in the way the code later assumes it is19:45
programmerjakeif it's not consistent, then of course budget-sync may give the wrong results19:46
programmerjakelkcl: let me know when you decide if the "Luke Kenneth Casson Leighton (donated)" user is ok or not19:51
programmerjakealso (unrelated), the csvs should probably have the full name of people rather than their abbreviation, that's what I'm trying to have as the keys in the people table in the toml19:53
programmerjakeadditionally, csv supports spaces in fields, you don't need to replace all spaces with underlines19:53
programmerjake#458 thx, i'll work on it soon19:54
lkclprogrammerjake, you've made a *public* declaration *removing* me from the list of identities that will receive the amounts paid to that moniker19:55
lkclthis is EXTREMELY serious and needs IMMEDIATE reversion19:55
lkclwith that moniker removed the generation of the CSV files which we send to NLnet for them to use to cross-reference their reconds will also have those payments REMOVED19:56
lkclthe fact that the "error" is not an "error" but is a warning in capital letters is nowhere near as critically important as making a false declaration to our funding body19:57
programmerjakeno...it just that there are two budget-sync users that both belong to you...it does *not* mean that you didn't get those payments or that I'm trying to say you didn't get those payments19:57
lkclyes, and that's important information19:57
programmerjakethat's only cuz you changed it to a warning...i originally put it as an error for a reason19:58
lkcland the program critically relies on - generates output - which shows that fact19:58
lkcli know19:58
lkcland reality is: the error is not an error19:58
lkclit is a fact that happens to be inconvenient for the program19:58
lkclplease revert it, this is extremely serious19:59
lkclthe "inconvenience" is as nothing compared to the seriousness of making a false declaration to NLnet19:59
lkcland to creating false records that are provided to them19:59
lkclby removing that 2nd identity, the CSV files now REMOVE all the payments under my name20:00
programmerjakeit's not a false declaration since it still attributes the payments to you...think of it as if you owned two bank accts and some payments went to one and some to the other20:00
lkclNLnet's Finance Director will look at the CSV files and go, "what the hell is going on, these were previously paid to lkcl, now what the hell is going on"20:00
lkclBob uses the CSV files to track records20:01
lkclplease do not argue about this: it is critically important.20:01
lkclplease understand and accept the severity of this mistake20:01
lkclplease revert it immediately20:01
lkclplease take this action immediately20:01
lkclsolutions can be discussed *after* reversion of this extremely serious critical error20:02
programmerjakewell, in any case it *was* overwriting previous payments so in some cases where you got paid eur 400 (300 to lkcl 100 to donated), it sees the donated one last so replaces in the csv the entry with the 100 it saw last20:02
programmerjakei don't thing reversion is the right approach, since that changes the csvs back to being silently broken rather then how they are now, explicit about how/why exactly you got paid20:05
lkclthen that should have been discussed BEFORE going ahead and making changes, rather than forcing me to deal with it in an urgent high-priority fashion, shouldn't it?20:07
lkclif something got overwritten that's a bug that's great to have found20:07
lkclbut *deliberately lying* to our funding body (via a program) has me absolutely freaking out.20:08
lkcli'm *really* not happy to be thinking about this under that kind of pressure20:08
lkclto the point where i'm finding it difficult to focus on what you're saying, due to the urgency20:09
lkclso please20:09
lkclcan you please20:09
lkcl*revert the changes*20:09
programmerjakeas long as we don't use the latest version to give nlnet csvs until it's satisfactorially fixed, there shouldn't be an urgent problem...20:09
programmerjakek, i'll revert them for now...20:09
lkcland then raise appropriate bugreports which allow it to be discussed much more carefully20:10
lkcli told NLnet themselves how to run the program20:10
lkclthey could run it any time, because it's public access20:10
lkcldon't get me wrong: it's a hugely important thing that you discovered there's a mistake (an overwrite)20:10
lkcli threw the CSV file system together in a hurry because Bob was quite pissed20:11
programmerjakeah, ok. i thought it was that you gave them csvs rather than them generating them themelves20:11
lkcli did but i also told them how to run it themselves20:12
programmerjakehttps://git.libre-soc.org/?p=utils.git;a=patch;h=2a7b0caa43c1c771fa0fdb425a35ac17a670942e20:14
programmerjakelkcl ^20:15
programmerjakemaybe we should email nlnet so they know I found the overwriting issue20:16
lkclyes, when i next do a set of CSVs to Bob.20:17
lkclcan you raise a bugreport about that one, with priority "critical"? highlight what you found (which bug overwrites which)20:18
programmerjakeask them if they're ok with splitting lkcl into two separate users20:18
programmerjakei also changed the makefile to have more useful options...build is now the default rather than upload20:19
programmerjakeso you'll have to run `make upload`20:19
programmerjakelkcl, we could probably just link nlnet to https://bugs.libre-soc.org/show_bug.cgi?id=70620:40
programmerjakeit has a summary of what happened20:40
kylellkcl, not having much luck utilizing soc test_runner21:42
lkclkylel, hmmm... let me see if i can chuck in a parameter that wipes out use of HDL23:20
lkcl1 sec23:20
programmerjakelkcl, for budget-sync, do you care if it depends on dict being ordered (rendering OrderedDict redundant) that requires cpython 3.6, python 3.7 or later iirc23:23
programmerjakeusing dict instead of OrderedDict will make it easier to write23:23
lkclOrderedDict creates specific ordering, which means that when doing "diff -u" on revisions, the changes will be detectable23:24
programmerjakedict does too...23:24
lkclah no it doesn't.23:24
programmerjakewell, you're wrong as of python 3.723:25
lkcldict ordering *completely* changes arbitrarily depending on its contents and the order in which things are added23:25
lkclintriguing...23:25
lkclhmm, so why does litex completely change the order of its parameters?23:25
lkcl(damn nuisance, that - committing code and having the parameters completely reverse order)23:26
lkcland move about, arbitrarily.23:26
programmerjakeas of python 3.7, dict order is strictly insertion order, just like OrderedDict23:26
lkclhttps://medium.com/junior-dev/python-dictionaries-are-ordered-now-but-how-and-why-5d5a40ee327f23:27
lkclapparently there's some differences, just reading up on that now23:27
lkcllet's see if it matters23:27
lkclok we need neither of the two additional functions, OrderedDict.reversed23:28
lkclor OrderedDict.move_to_end23:28
lkclso yes, dict is perfectly fine23:28
lkclyay, timesaving23:28
programmerjakewell, as of python 3.8, reversed works with dict23:28
lkclnice!23:29
lkclkylel, done.23:31
programmerjakefor ordered set, we can just use a dict that always has values of None23:32
lkclblech :)23:33
lkclthat was the argument used i believe to justify the insanity of removing set() 8-10 years ago :)23:33
lkclbut set() has difference and union and other extremely useful functions, thank goodness common sense prevailed23:34
programmerjakemakes me wish set was ordered by default...23:34
lkclyyeahhh...23:34
* lkcl totally agrees23:34
lkclkylel, i will add a similar parameter for sim as well23:35
programmerjakerust has the best of both worlds...HashSet/HashMap for speed, BTreeSet/BTreeMap for speed and order23:35
kylelcool, i'll be interested to see what you did because because it seemed like they needed each other somewhat in there.23:38
lkclyes, you can't not have a Simulator object23:39
lkcleven if it's not actually used23:39
lkclas in, doesn't run ISACaller23:39
lkclkylel: done23:40
lkclvery straightforward.23:40
lkclhowever, it neatly highlights the pieces that need to be thrown behind StateRunner23:41
lkclhttps://git.libre-soc.org/?p=openpower-isa.git;a=commitdiff;h=f4e1d1c87234b3d8cf6a9d2d7db2b09082d3b2cc23:41
lkclkylel, more comments.23:50
kyleli was closer than I thought :)23:50
lkcl:)23:51
lkclkylel, i suggest going one function at a time.23:55
lkcllike, start with e.g. SimRunner, creating an object that has just *one* function23:56
lkclor, not even one function, but only a constructor23:56
lkclso it contains the simdec2 for example23:57

Generated by irclog2html.py 2.17.1 by Marius Gedminas - find it at https://mg.pov.lt/irclog2html/!