kylel | lkcl, did you want me to work on expected alu cases or is something else required? | 13:41 |
---|---|---|
lkcl | kylel, as a "default background" thing, yes, test_caller_shift_rot.py | 13:44 |
lkcl | what would be really good would be to convert test_caller_shift_rot.py to be in the same format as test/alu/alu_cases.py | 13:45 |
lkcl | now that they both support "ExpectedState", this should be fairly easy | 13:45 |
kylel | ok, will take a look. Want a new bug for this? | 13:46 |
lkcl | that's another "step along the way" | 13:46 |
* lkcl thinks | 13:46 | |
lkcl | yyeah the existing one is getting pretty big (comment-wise) | 13:46 |
lkcl | and i'd say it's a good "stake in the ground" for preliminary investigation | 13:46 |
kylel | link it to 686? | 13:50 |
lkcl | yes | 13:51 |
lkcl | otherwise it becomes impossible to find related things | 13:51 |
lkcl | always important to link to at least... something | 13:52 |
lkcl | then, next time you look at it, there's an extra related bugreport 2 degrees or 3 degrees away that turns out to be immediately relevant | 13:52 |
lkcl | but if isolated it's virtually impossible to find anything | 13:52 |
lkcl | kylel, you can see i added one, here: https://git.libre-soc.org/?p=openpower-isa.git;a=commitdiff;h=907570fdb16fa0baf737a79017b28ce4b980a926 | 13:57 |
lkcl | but basically, what i suggest is: | 13:58 |
lkcl | * get test_caller_shift_rot.py to derive from TestAccumulatorBase | 13:58 |
lkcl | * copy how things are done in test_issuer.py (suite.addTest) | 13:59 |
lkcl | * create your own TestRunner which calls run_tst_program | 14:00 |
lkcl | the way that TestAccumulatorBase works is: | 14:00 |
lkcl | it accumulates a list of tests (into test_data) | 14:00 |
lkcl | then you can set up **ONE** Simulator (and one PowerDecoder2, which is very expensive / timeconsuming) | 14:00 |
lkcl | and in a for-loop run *ALL* the instances accumulated in test_data | 14:01 |
lkcl | behind a self.subTest(test.name) | 14:01 |
lkcl | the function which does that needs to be named "run_all() | 14:01 |
lkcl | " | 14:01 |
lkcl | https://git.libre-soc.org/?p=soc.git;a=blob;f=src/soc/simple/test/test_runner.py;h=045e96813767e2be0e0920934bc0f500f92b54e1;hb=f7b708268bf758a2cea294fd7f070be4d78acc6b#l256 | 14:02 |
lkcl | actually... | 14:02 |
lkcl | hilariously... | 14:02 |
lkcl | you could probably take test_runner.py as-is and literally cut out the call to run_hdl_state()! | 14:03 |
lkcl | cut out TestIssuerInternal at line 292 | 14:03 |
lkcl | etc. etc. | 14:03 |
lkcl | if you wanted to make it a bit "cleverer", make it *optional* - in TestRunner itself - to activate TestIssuer | 14:04 |
lkcl | which might be simpler | 14:04 |
kylel | guess we'll see :) | 14:10 |
lkcl | actually, that should be a brain-dead-simple way to do it | 14:19 |
lkcl | because effectively it's duplicated code, if you compare TestRunner.run_all to the run_tst function | 14:20 |
* lkcl hmmms and deliberates, a candle instead of a lightbulb above my head... | 19:31 | |
lkcl | i'm thinking of a new API, kylel, one which has a "Setup" function, "Runner" function, "Prepare for Test" function, and so on | 19:33 |
lkcl | err exactly like how python unit tests work, in fact | 19:33 |
lkcl | https://docs.python.org/3/library/unittest.html | 19:33 |
lkcl | import unittest | 19:34 |
lkcl | class WidgetTestCase(unittest.TestCase): | 19:34 |
lkcl | def setUp(self): | 19:34 |
lkcl | self.widget = Widget('The widget') | 19:34 |
programmerjake | i'd recommend using the unittest api, rather than rewriting it | 19:36 |
lkcl | yyeah 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 on | 19:37 |
lkcl | and, there's no concept "step-to-next-simulation-state" | 19:37 |
lkcl | programmerjake, you saw about the (severe) impact that removing me from the "donated" list has? | 19:38 |
lkcl | that's an actual public declaration, "this money was NOT paid to lkcl" | 19:39 |
lkcl | which is extremely serious. | 19:39 |
lkcl | i run budget-sync by looking at the output, each and every time. | 19:39 |
lkcl | i never ignore warnings. | 19:39 |
programmerjake | write a class that inherits from the unittest api, you can add methods for stepping simulator state | 19:40 |
programmerjake | lkcl: 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 |
programmerjake | it should still be obvious that it's still lkcl | 19:42 |
programmerjake | you ignored errors by changing the code to instead print NOT AN ERROR... | 19:44 |
programmerjake | i 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 is | 19:45 |
programmerjake | if it's not consistent, then of course budget-sync may give the wrong results | 19:46 |
programmerjake | lkcl: let me know when you decide if the "Luke Kenneth Casson Leighton (donated)" user is ok or not | 19:51 |
programmerjake | also (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 toml | 19:53 |
programmerjake | additionally, csv supports spaces in fields, you don't need to replace all spaces with underlines | 19:53 |
programmerjake | #458 thx, i'll work on it soon | 19:54 |
lkcl | programmerjake, you've made a *public* declaration *removing* me from the list of identities that will receive the amounts paid to that moniker | 19:55 |
lkcl | this is EXTREMELY serious and needs IMMEDIATE reversion | 19:55 |
lkcl | with 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 REMOVED | 19:56 |
lkcl | the 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 body | 19:57 |
programmerjake | no...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 payments | 19:57 |
lkcl | yes, and that's important information | 19:57 |
programmerjake | that's only cuz you changed it to a warning...i originally put it as an error for a reason | 19:58 |
lkcl | and the program critically relies on - generates output - which shows that fact | 19:58 |
lkcl | i know | 19:58 |
lkcl | and reality is: the error is not an error | 19:58 |
lkcl | it is a fact that happens to be inconvenient for the program | 19:58 |
lkcl | please revert it, this is extremely serious | 19:59 |
lkcl | the "inconvenience" is as nothing compared to the seriousness of making a false declaration to NLnet | 19:59 |
lkcl | and to creating false records that are provided to them | 19:59 |
lkcl | by removing that 2nd identity, the CSV files now REMOVE all the payments under my name | 20:00 |
programmerjake | it'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 other | 20:00 |
lkcl | NLnet'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 |
lkcl | Bob uses the CSV files to track records | 20:01 |
lkcl | please do not argue about this: it is critically important. | 20:01 |
lkcl | please understand and accept the severity of this mistake | 20:01 |
lkcl | please revert it immediately | 20:01 |
lkcl | please take this action immediately | 20:01 |
lkcl | solutions can be discussed *after* reversion of this extremely serious critical error | 20:02 |
programmerjake | well, 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 last | 20:02 |
programmerjake | i 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 paid | 20:05 |
lkcl | then 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 |
lkcl | if something got overwritten that's a bug that's great to have found | 20:07 |
lkcl | but *deliberately lying* to our funding body (via a program) has me absolutely freaking out. | 20:08 |
lkcl | i'm *really* not happy to be thinking about this under that kind of pressure | 20:08 |
lkcl | to the point where i'm finding it difficult to focus on what you're saying, due to the urgency | 20:09 |
lkcl | so please | 20:09 |
lkcl | can you please | 20:09 |
lkcl | *revert the changes* | 20:09 |
programmerjake | as 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 |
programmerjake | k, i'll revert them for now... | 20:09 |
lkcl | and then raise appropriate bugreports which allow it to be discussed much more carefully | 20:10 |
lkcl | i told NLnet themselves how to run the program | 20:10 |
lkcl | they could run it any time, because it's public access | 20:10 |
lkcl | don't get me wrong: it's a hugely important thing that you discovered there's a mistake (an overwrite) | 20:10 |
lkcl | i threw the CSV file system together in a hurry because Bob was quite pissed | 20:11 |
programmerjake | ah, ok. i thought it was that you gave them csvs rather than them generating them themelves | 20:11 |
lkcl | i did but i also told them how to run it themselves | 20:12 |
programmerjake | https://git.libre-soc.org/?p=utils.git;a=patch;h=2a7b0caa43c1c771fa0fdb425a35ac17a670942e | 20:14 |
programmerjake | lkcl ^ | 20:15 |
programmerjake | maybe we should email nlnet so they know I found the overwriting issue | 20:16 |
lkcl | yes, when i next do a set of CSVs to Bob. | 20:17 |
lkcl | can you raise a bugreport about that one, with priority "critical"? highlight what you found (which bug overwrites which) | 20:18 |
programmerjake | ask them if they're ok with splitting lkcl into two separate users | 20:18 |
programmerjake | i also changed the makefile to have more useful options...build is now the default rather than upload | 20:19 |
programmerjake | so you'll have to run `make upload` | 20:19 |
programmerjake | lkcl, we could probably just link nlnet to https://bugs.libre-soc.org/show_bug.cgi?id=706 | 20:40 |
programmerjake | it has a summary of what happened | 20:40 |
kylel | lkcl, not having much luck utilizing soc test_runner | 21:42 |
lkcl | kylel, hmmm... let me see if i can chuck in a parameter that wipes out use of HDL | 23:20 |
lkcl | 1 sec | 23:20 |
programmerjake | lkcl, 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 iirc | 23:23 |
programmerjake | using dict instead of OrderedDict will make it easier to write | 23:23 |
lkcl | OrderedDict creates specific ordering, which means that when doing "diff -u" on revisions, the changes will be detectable | 23:24 |
programmerjake | dict does too... | 23:24 |
lkcl | ah no it doesn't. | 23:24 |
programmerjake | well, you're wrong as of python 3.7 | 23:25 |
lkcl | dict ordering *completely* changes arbitrarily depending on its contents and the order in which things are added | 23:25 |
lkcl | intriguing... | 23:25 |
lkcl | hmm, 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 |
lkcl | and move about, arbitrarily. | 23:26 |
programmerjake | as of python 3.7, dict order is strictly insertion order, just like OrderedDict | 23:26 |
lkcl | https://medium.com/junior-dev/python-dictionaries-are-ordered-now-but-how-and-why-5d5a40ee327f | 23:27 |
lkcl | apparently there's some differences, just reading up on that now | 23:27 |
lkcl | let's see if it matters | 23:27 |
lkcl | ok we need neither of the two additional functions, OrderedDict.reversed | 23:28 |
lkcl | or OrderedDict.move_to_end | 23:28 |
lkcl | so yes, dict is perfectly fine | 23:28 |
lkcl | yay, timesaving | 23:28 |
programmerjake | well, as of python 3.8, reversed works with dict | 23:28 |
lkcl | nice! | 23:29 |
lkcl | kylel, done. | 23:31 |
programmerjake | for ordered set, we can just use a dict that always has values of None | 23:32 |
lkcl | blech :) | 23:33 |
lkcl | that was the argument used i believe to justify the insanity of removing set() 8-10 years ago :) | 23:33 |
lkcl | but set() has difference and union and other extremely useful functions, thank goodness common sense prevailed | 23:34 |
programmerjake | makes me wish set was ordered by default... | 23:34 |
lkcl | yyeahhh... | 23:34 |
* lkcl totally agrees | 23:34 | |
lkcl | kylel, i will add a similar parameter for sim as well | 23:35 |
programmerjake | rust has the best of both worlds...HashSet/HashMap for speed, BTreeSet/BTreeMap for speed and order | 23:35 |
kylel | cool, i'll be interested to see what you did because because it seemed like they needed each other somewhat in there. | 23:38 |
lkcl | yes, you can't not have a Simulator object | 23:39 |
lkcl | even if it's not actually used | 23:39 |
lkcl | as in, doesn't run ISACaller | 23:39 |
lkcl | kylel: done | 23:40 |
lkcl | very straightforward. | 23:40 |
lkcl | however, it neatly highlights the pieces that need to be thrown behind StateRunner | 23:41 |
lkcl | https://git.libre-soc.org/?p=openpower-isa.git;a=commitdiff;h=f4e1d1c87234b3d8cf6a9d2d7db2b09082d3b2cc | 23:41 |
lkcl | kylel, more comments. | 23:50 |
kylel | i was closer than I thought :) | 23:50 |
lkcl | :) | 23:51 |
lkcl | kylel, i suggest going one function at a time. | 23:55 |
lkcl | like, start with e.g. SimRunner, creating an object that has just *one* function | 23:56 |
lkcl | or, not even one function, but only a constructor | 23:56 |
lkcl | so it contains the simdec2 for example | 23:57 |
Generated by irclog2html.py 2.17.1 by Marius Gedminas - find it at https://mg.pov.lt/irclog2html/!