| *** doppo <doppo!~doppo@2604:180::e0fc:a07f> has quit IRC | 03:20 | |
| *** doppo <doppo!~doppo@2604:180::e0fc:a07f> has joined #libre-soc | 03:20 | |
| lkcl- | ghostmansd, sorry, i am back from business meetings. please can you remove the use of dataclass from openpower-isa. | 04:08 |
|---|---|---|
| lkcl- | it was introduced by jacob because of his insistence on turning python into rust and i have made it abundantly clear to him multiple times that this is not okay. | 04:08 |
| lkcl- | also i do not want to have to explain to HDL engineers the advanced features of python when they are already going to be overwhelmed with the concept of object-orientated programming and the abstraction | 04:10 |
| lkcl- | every new advanced feature of python that has to be explained to a team of HDL engineers as part of their training is a delay and a risk to the project | 04:12 |
| lkcl- | a factory which instantiates on-demand-dynamic classes with member variables based on deriving from object() and using the three arguments of the type() function to give the dynamic-class a name | 04:19 |
| lkcl- | then automatically adding instances to it from a dictionary (fieldname, fieldtype) i have no problem with | 04:19 |
| lkcl- | (as long as it's well-documented in the comments what the hell's going on) | 04:20 |
| lkcl- | because type() has literally been around for... 30 years now? | 04:20 |
| programmerjake | imho who cares how long it's been around, that doesn't mean it's more or less user-friendly -- in this case calling type to create new classes is imho quite user-unfriendly. the liskof substitution principle is imho not a good reason to use Python, it only makes the code less clear and more fragile and harder to understand. | 04:25 |
| programmerjake | there's a reason TypeScript came to dominate the web -- because it fixes the main problem in Javascript -- lack of typing. lkcl-style Python has the same major flaw. | 04:25 |
| programmerjake | and, no, this isn't about making python into rust. it's about making python more understandable when you read/write/use the code. | 04:26 |
| programmerjake | because dataclasses are shorter, less bug prone, and easier to understand than writing out a class with the corresponding init, comparison, hash, repl, etc. methods | 04:27 |
| programmerjake | to be clear, i'm arguing dataclasses should be allowed, not that type annotations in general should be | 04:29 |
| programmerjake | also, the concept of structs -- dataclasses in python, is already well known by most hdl engineers -- they're often used in systemverilog and vhdl as well as most programming languages | 04:31 |
| programmerjake | they're highly likely to be more familiar than writing out the equivalent python by hand | 04:32 |
| programmerjake | well, i'll let you think about that, gtg now. | 04:33 |
| lkcl- | programmerjake, tough. | 04:36 |
| lkcl- | i did not realise that dataclasses specify types. | 04:36 |
| lkcl- | on learning this i am instigating a ban on the use of dataclass in libre-soc HDL. | 04:37 |
| lkcl- | effective immediate | 04:37 |
| lkcl- | please begin converting and migrating all HDL to non-dataclass such that the import and use in setup.py may be removed. | 04:37 |
| lkcl- | this is not negotiable. | 04:38 |
| programmerjake | uuh, they're part of python's stdlib, not a package from setup.py | 04:38 |
| lkcl- | python 3.7 | 04:38 |
| lkcl- | that is the standard version that is used by this project. | 04:39 |
| programmerjake | it was added in python 3.7 | 04:39 |
| lkcl- | if i had known that dataclasses specify types then i would have immediately said NO back months ago | 04:39 |
| programmerjake | https://peps.python.org/pep-0557/ | 04:40 |
| lkcl- | programmerjake, the answer remains NO. | 04:40 |
| lkcl- | "not negotiable" means NOT NEGOTIABLE. | 04:40 |
| lkcl- | please begin planning the migration process of removing dataclasses from all libre-soc HDL. | 04:41 |
| lkcl- | nmigen on the other hand is a completely different matter, where the formal proofs and use of smtlib2 is very clear as to why it is beneficial | 04:42 |
| lkcl- | i will explain why when it is not 4:42am and i am dizzy and still partly in shock from a traumatic car journey | 04:43 |
| programmerjake | i don't see how it's beneficial in nmigen but not in other code...in any case i can't work on this right now, i'm unavailable till tuesday | 04:44 |
| programmerjake | please create a bug for it. | 04:44 |
| programmerjake | ttyl | 04:46 |
| programmerjake | one last thing: according to the 2021 python survey, 74% of python devs use optional type hinting | 05:04 |
| *** markos <markos!~Konstanti@ppp005054209140.access.hol.gr> has quit IRC | 08:55 | |
| *** markos <markos!~Konstanti@ppp005054209140.access.hol.gr> has joined #libre-soc | 09:09 | |
| *** markos <markos!~Konstanti@ppp005054209140.access.hol.gr> has quit IRC | 09:23 | |
| ghostmansd | lkcl, what happened with the car journey? Is everything OK? | 10:00 |
| ghostmansd | > please can you remove the use of dataclass from openpower-isa. | 10:02 |
| ghostmansd | > it was introduced by jacob because of his insistence on turning python into rust | 10:03 |
| ghostmansd | Apparently we've arrived to the same conclusion with Jacob without even looking at each other's work. | 10:03 |
| lkcl- | ghostmansd, i had only -1.5 diopter glasses and my eyes are -3.5 | 10:39 |
| ghostmansd | Whoa. Were you driving? | 10:40 |
| lkcl- | they've also gone "prism" from looking at computer screens. end-result, in the dark, a 2 hour journey, was absolute hell | 10:40 |
| lkcl- | a 26-year-old toyota starlight with a windscreen that has the usual abrasions on it which causes oncoming car headlights to create circular hairline haze around them | 10:41 |
| lkcl- | and of course people now have these fucking stupid xeon point-source headlights now | 10:41 |
| lkcl- | end result is i had to close one eye in order to not see double, pick a lorry to get behind, hold up one hand in front of me to block out both the side mirror *and* the oncoming traffic headlight glare | 10:42 |
| lkcl- | the outbound journey was absolutely fine as it was during the day. on the way back i pretty much literally had to drive by memory of the road turns. | 10:44 |
| lkcl- | anyway. | 10:44 |
| lkcl- | class Instruction is basically what PowerOp is | 10:45 |
| lkcl- | https://git.libre-soc.org/?p=openpower-isa.git;a=blob;f=src/openpower/decoder/power_decoder.py;h=2b4799c6dcf53a43879a167250d52389bd83ee7a;hb=9dbf16f3aa783d95ca92ad74ebef9474c7750dd2#l179 | 10:45 |
| lkcl- | except PowerOp is a nmigen-based "thing" that contains what-is-needed-which-comprises-the-instruction-information | 10:46 |
| lkcl- | what you're doing with dataclasses *already exists* in the form of the power_op_types | 10:47 |
| lkcl- | 122 power_op_types = {'function_unit': Function, | 10:47 |
| lkcl- | 123 'internal_op': MicrOp, | 10:47 |
| lkcl- | 124 'form': Form, | 10:47 |
| lkcl- | and the conversion from CSV columns to field-aka-member-variable-names is already also done in the form of power_op_csvmap | 10:48 |
| lkcl- | 147 power_op_csvmap = {'function_unit': 'unit', | 10:48 |
| lkcl- | 148 'form': 'form', | 10:48 |
| lkcl- | 149 'internal_op': 'internal op', | 10:48 |
| lkcl- | the quirky bit is there are some fields that are single-bit, these went into a list single_bit_flags | 10:49 |
| lkcl- | 201 for bit in single_bit_flags: | 10:49 |
| lkcl- | the bit that is slightly confusing in PowerDecoder is that there is a function PowerOp.like() | 10:53 |
| lkcl- | therefore it becomes possible to only have this | 10:53 |
| lkcl- | 342 self.op = PowerOp(name=name, subset=col_subset) | 10:53 |
| ghostmansd | Luke, this is exactly waht's wrapped in the class. The single_bit_flags are present as Flags. | 10:53 |
| lkcl- | and from that point onwards only have to call, within loops, to create instruction Records: op.append(self.op.like(xxx)) | 10:54 |
| lkcl- | yes, indeed, but you're duplicating PowerOp [and using dataclasses] | 10:54 |
| ghostmansd | You have the code split here and there, with multiple copies. CSVs are handled multiple times. | 10:54 |
| ghostmansd | Yes, this is exactly what I did, except that I added paranoid checks which were missed, cleaned the code and put it in one place. | 10:55 |
| ghostmansd | As I explained, I intend to migrate the whole code to these classes. | 10:55 |
| lkcl- | ok - as long as they do not use dataclass. | 10:55 |
| ghostmansd | But I cannot do it in one shot, the migration will take a while. | 10:55 |
| ghostmansd | Now to the dataclasses. I need the rationale. | 10:56 |
| lkcl- | and as long as you also understand that PowerOp takes subset-of-fields | 10:56 |
| lkcl- | the reason for that is that we absolutely cannot have full decoders | 10:56 |
| ghostmansd | It's not a problem to convert the dataclass to subset of fields. | 10:56 |
| lkcl- | hang on give me a second. this is quite involved | 10:56 |
| lkcl- | we have a main decoder which identifies the instruction by its type, enough to identify its registers, what the OP_xxx is, what the unit is | 10:57 |
| lkcl- | but we also have about 10 pipelines | 10:57 |
| lkcl- | the number of wires created by a full decode would be somewhere around 2,000 | 10:58 |
| lkcl- | and the number of gates of the order of 10,000 | 10:58 |
| lkcl- | 10,000 x 10 pipelines is 100,000 gates, with 20,000 wires fanning out in all directions | 10:58 |
| lkcl- | this is clearly not ok | 10:58 |
| lkcl- | so i added two things: | 10:58 |
| lkcl- | 1) a row filter to cut down the number of entries in "Satellite" PowerDecoders | 10:59 |
| lkcl- | 2) a column filter to cut down the number of entries in the PowerOp() instances to *only* those fields that were actually needed | 10:59 |
| lkcl- | the class suites *need* to recognise that. | 11:00 |
| lkcl- | the VLSI tools are not intelligent enough to recognise, "oh, you have not used 80% of these wires, let me delete them automatically for you", they are left dangling | 11:00 |
| ghostmansd[m] | And the problem to recognizing this from accepting Database on input is?... | 11:01 |
| lkcl- | so we *have* to, on reading the CSV files, only create class-instances of what exactly is needed | 11:01 |
| lkcl- | none - it's quite straightforward, but not exactly obvious as to why it's needed | 11:01 |
| ghostmansd[m] | It's needed because I want to have a clear and explicit common interface, used by all subsystems, as a single source of truth. | 11:02 |
| ghostmansd[m] | No, the stuff mixed with Signal, and not checked at all, is not a suitable candidate. | 11:03 |
| lkcl- | now, i have no problem at all with creating an iterator which creates on-demand filtered instances | 11:03 |
| lkcl- | remember, i mentioned that we planned to abstract out the use of Signal() and Record() and remove dependence on nmigen but ran out of time to do so | 11:03 |
| lkcl- | we're effectively doing that now | 11:04 |
| ghostmansd[m] | Yes, and I used the time I had to do exactly this. Since our code basically uses get_svp64_csv, I wrapped it. | 11:04 |
| ghostmansd[m] | The problem is, it does wraps this several times. | 11:05 |
| ghostmansd[m] | With no checks, lot of assumptions and a copy&paste. | 11:05 |
| lkcl- | well... it sort-of does and sort-of doesn't. the... yes. | 11:05 |
| lkcl- | things evolved rapidly, it's time to do the "step back and rationalise" | 11:06 |
| ghostmansd[m] | Yes. | 11:06 |
| lkcl- | let me just put in the comment into the use of PowerOp | 11:06 |
| ghostmansd[m] | I don't see an issue with dataclass per se. It's the same as the named tuple, except that this is dict a d that is, well, named tuple. | 11:06 |
| lkcl- | it's out. i cannot explain | 11:07 |
| ghostmansd[m] | The dataclass allows to change repr and add some methods/properties atop, and that's all. | 11:07 |
| lkcl- | to 20-50 HDL engineers who have never even been introduced to Object-Orientated Programming what it is | 11:07 |
| lkcl- | no. | 11:07 |
| lkcl- | it's just not appropriate | 11:08 |
| ghostmansd[m] | So you expect these to understand the gory fuck in tree_analyze, but not enter two words in Google? | 11:08 |
| lkcl- | no, i don't :) | 11:08 |
| ghostmansd[m] | This doesn't constitutes a valid technical rationale. | 11:08 |
| ghostmansd[m] | This is opinionated. | 11:08 |
| lkcl- | it'll take some time to explain | 11:09 |
| ghostmansd[m] | You seem to think dataclass is a struct. | 11:09 |
| lkcl- | technically it's just not the right solution | 11:09 |
| ghostmansd[m] | In fact, it isn't. | 11:09 |
| lkcl- | it's a static allocation of typed-fields where because of the way that PowerOp works we need a *dynamic* allocation not of typed-fields but dynamic allocation of Member instances | 11:10 |
| ghostmansd[m] | These are not typed. | 11:10 |
| ghostmansd[m] | Again, you're thinking of struct | 11:10 |
| ghostmansd[m] | Dataclasses are not structures. | 11:11 |
| ghostmansd[m] | There's nothing "typed" here. | 11:11 |
| ghostmansd[m] | It's as typed as your dict with key:type mapping. | 11:11 |
| ghostmansd[m] | You're attributing to dataclass the logic it doesn't have. | 11:12 |
| lkcl- | i am still recovering from the journey and also catching up. i need to take a break (already). if you can please use either setattr or self.__dict__[fieldname] = fieldinstance_from_csv | 11:13 |
| lkcl- | similar to how things work in PowerOp | 11:13 |
| lkcl- | then i'm perfectly happy | 11:13 |
| ghostmansd[m] | This is what dataclass is. | 11:13 |
| lkcl- | no, it really isn't. | 11:13 |
| ghostmansd[m] | Dataclass(x) == cls(x).__dict__ | 11:13 |
| ghostmansd[m] | Yes it really _is_. Discarding the sugar, it is a dict. | 11:14 |
| lkcl- | the need for the class to be declared - at all - is bypassed by setattr | 11:14 |
| ghostmansd[m] | setattr works on any objects | 11:16 |
| lkcl- | phone call 1 sec | 11:16 |
| lkcl- | yes exactly it does. | 11:17 |
| ghostmansd[m] | Including dataclasses. | 11:17 |
| ghostmansd[m] | Luke, before proceeding further, I need a valid explanation. Dataclasses, compared to "some dict I got from parsing", document what's here, and give a huge hint how the data is expected to look like. They don't enforce any other restrictions (well, except that they can also be marked as "immutable"). The last one is actually a bonus, because what you did to the records in get_svp64_csv is barely readable, you basically | 11:23 |
| ghostmansd[m] | mix different unrelated records into the huge record, add your own keys and so forth. It makes it virtually impossible to realize how the record is created without several days of reading the code. Yes, unlike the existing code, dataclass-per-CSV documents the fields and the layout. | 11:23 |
| lkcl- | i expect Instruction *instances* to have the same fields as PowerOp | 11:25 |
| lkcl- | but *not* to have Signal()s | 11:25 |
| ghostmansd[m] | And then it's clear how we calculate each and every piece of information. And how all CSVs are merged to a single entity called Instruction. | 11:25 |
| lkcl- | just the raw numbers, bits, or Enums that need to go *into* a PowerOp | 11:26 |
| ghostmansd[m] | And the problem of getting this information from Instruction is?... | 11:26 |
| ghostmansd[m] | Again: you can have a method which gets the job done, or custom map function, or whatever. | 11:27 |
| lkcl- | plus enough meta-information such that it is possible to pass an instance of an Instruction() to the constructor of PowerOp and have PowerOp *create* the Signal()s it needs | 11:27 |
| ghostmansd[m] | Again: Instruction _has_ all the information virtually available. | 11:27 |
| lkcl- | ok, so what the perceived advantage of dataclasses is, is that there Shall Be a class by which it looks like... | 11:27 |
| lkcl- | except it has *too much* information. | 11:27 |
| lkcl- | 1 sec | 11:27 |
| ghostmansd[m] | What does it mean "too much"? | 11:28 |
| lkcl- | 316 svp64: SVP64Record | 11:28 |
| lkcl- | that must be removed | 11:28 |
| ghostmansd[m] | Why so? This is a link to SVP64 record. | 11:28 |
| lkcl- | because if the PowerDecoder (once coverted to use Instruction) is only used to create 32-bit scalar instructions.... | 11:28 |
| lkcl- | 317 rc: bool = False | 11:28 |
| lkcl- | that must be removed | 11:29 |
| lkcl- | because some of the Satellite PowerDecoders *do not need it* | 11:29 |
| ghostmansd[m] | SP64Record can be None | 11:29 |
| lkcl- | there exists pipelines that have no actual arithmetic operations and consequently cannot, should not, and must not, even have any knowledge of Rc at all | 11:30 |
| ghostmansd[m] | This is a hint to whether the instruction name should end with dot. | 11:30 |
| ghostmansd[m] | Actually, names. | 11:30 |
| lkcl- | which there are some instructions that do not have that at all | 11:30 |
| ghostmansd[m] | This is not RC.RC | 11:31 |
| ghostmansd[m] | But OK, I can drop that one, simply changing the name. | 11:31 |
| lkcl- | mmm... ok. | 11:31 |
| lkcl- | that's a valid point | 11:31 |
| lkcl- | but the svp64 record is not | 11:32 |
| ghostmansd[m] | Again, SVP64 record is nullable | 11:32 |
| ghostmansd[m] | This should have been marked appropriately. | 11:32 |
| lkcl- | but for people who create Scalar Power ISA they should not even know it exists | 11:32 |
| lkcl- | this is about "information cleanliness" | 11:32 |
| ghostmansd[m] | I can simply have a copy of fields there. | 11:33 |
| lkcl- | (for which OO was designed as you know) | 11:33 |
| ghostmansd[m] | This is for simplicity. | 11:33 |
| lkcl- | i do not like the declaration of the types, i do not like that they are statically declared | 11:33 |
| lkcl- | i can see *why* they have been declared statically | 11:33 |
| lkcl- | because it is normal to go to a class declaration in order to see what is there | 11:34 |
| lkcl- | and "understand", from a static reading of the code, what it is intended to do | 11:34 |
| ghostmansd[m] | Luke, ok, then drop power_op_types. | 11:34 |
| ghostmansd[m] | This is the same. | 11:34 |
| lkcl- | that's why dataclass really exists: to help simplify *static* understanding of code | 11:34 |
| lkcl- | 1 sec.. | 11:34 |
| ghostmansd[m] | Again: you're thinking of these as the types | 11:35 |
| lkcl- | we're talking at cross-purposes | 11:35 |
| lkcl- | power_op_types exists in order to be able to instantiate what is needed for PowerOp instances [aka, at some level, Instruction instances] | 11:35 |
| lkcl- | but do you notice that power_op_types is *not* a member of PowerOp itself? | 11:36 |
| ghostmansd[m] | Exactly! | 11:36 |
| ghostmansd[m] | And with dataclass, it serves the same purpose. | 11:36 |
| lkcl- | you see how they work together but are separate? | 11:36 |
| ghostmansd[m] | Check the factory methods. | 11:36 |
| lkcl- | no, it really doesn't | 11:36 |
| lkcl- | i know you have the factory methods | 11:36 |
| lkcl- | but you also have | 11:37 |
| lkcl- | 314 section: Section | 11:37 |
| lkcl- | 315 ppc: PPCRecord | 11:37 |
| lkcl- | 316 svp64: SVP64Record | 11:37 |
| lkcl- | 317 rc: bool = False | 11:37 |
| ghostmansd[m] | It really does. Again: these are not types, but hints. | 11:37 |
| ghostmansd[m] | And factories use these hints in order to be able to instantiate. | 11:37 |
| lkcl- | gimme a second.... | 11:38 |
| lkcl- | 123 class PPCRecord: | 11:38 |
| lkcl- | 147 opcode: Opcode | 11:38 |
| lkcl- | 148 comment: str | 11:38 |
| lkcl- | 149 flags: Flags = Flags() | 11:38 |
| lkcl- | 150 comment2: str = "" | 11:38 |
| lkcl- | no. | 11:38 |
| lkcl- | all those have to go | 11:38 |
| *** ghostmansd <ghostmansd!~ghostmans@broadband-188-32-220-156.ip.moscow.rt.ru> has quit IRC | 11:39 | |
| lkcl- | if they can be done dynamically (by a dict similar-or-identical-to power_op_types) i will be satisfied | 11:39 |
| lkcl- | at which point, what i expect is that you would go, "well that being the case we don't need dataclass at all" | 11:39 |
| lkcl- | which is the point of the exercise, here | 11:40 |
| lkcl- | these need to go | 11:40 |
| lkcl- | 202 regtype: _SVExtraRegType = _SVExtraRegType.NONE | 11:40 |
| lkcl- | 203 reg: _SVExtraReg = _SVExtraReg.NONE | 11:40 |
| lkcl- | and these | 11:40 |
| lkcl- | 236 identifier: str | 11:40 |
| lkcl- | 237 ptype: _SVPtype = _SVPtype.NONE | 11:40 |
| lkcl- | 238 etype: _SVEtype = _SVEtype.NONE | 11:40 |
| lkcl- | 239 in1: _In1Sel = _In1Sel.NONE | 11:40 |
| lkcl- | 240 in2: _In2Sel = _In2Sel.NONE | 11:40 |
| lkcl- | and these | 11:40 |
| lkcl- | 293 path: _pathlib.Path | 11:40 |
| lkcl- | 294 opcode: Opcode | 11:40 |
| lkcl- | 295 bitsel: BitSel | 11:40 |
| ghostmansd[m] | This is idiotic. You basically want to reinvent dataclass. | 11:41 |
| lkcl- | all to be replaced with some sort of list-of-tuples, or a dict, or basically literally anything other than "types" | 11:41 |
| ghostmansd[m] | Again: this is instantiated from dict. | 11:41 |
| lkcl- | i know it sounds idiotic | 11:41 |
| ghostmansd[m] | As simple as **dict_like_crap. | 11:41 |
| lkcl- | i don't have a problem at all with "reinventing" dataclass | 11:41 |
| ghostmansd[m] | Plus power_op_types atop. | 11:41 |
| lkcl- | as long as there is no "x colon y" in the code | 11:42 |
| ghostmansd[m] | I have, because it serves no useful purpose. | 11:42 |
| ghostmansd[m] | Again, Luke: this is not a valid technical argument. | 11:42 |
| ghostmansd[m] | Personal dislike for "x colon y" does not constitute a valid technical argument. | 11:42 |
| lkcl- | this is a communication flaw | 11:42 |
| ghostmansd[m] | Record's are CSV 1:1 mapping, an internal representation. | 11:43 |
| lkcl- | i have an intuitive feeling here having taken in multiple factors into consideration that i am not able to express right now, right at this minute, right in this conversation | 11:43 |
| lkcl- | for which dataclasses are and will always be an inappropriate solution | 11:43 |
| lkcl- | and i simply cannot express, right now, exactly what those are | 11:44 |
| ghostmansd[m] | I have time, no need to do it right now. | 11:44 |
| lkcl- | it will take me several weeks to go over it | 11:44 |
| lkcl- | unfortunately, i don't | 11:44 |
| lkcl- | we are ramping up the business side and much of it is confidential and i can't tell you anyway | 11:45 |
| lkcl- | we just had a Director's meeting of RED Semiconductor | 11:45 |
| lkcl- | what i am saying is, i need you to trust me that dataclasses are not appropriate | 11:45 |
| lkcl- | i'm dealing with too much at the moment to spare the several weeks to go over this | 11:46 |
| lkcl- | which ends up delaying what is already a complex conversion process for which funding is hard-terminated around the 2nd week of October | 11:47 |
| lkcl- | and we've been specifically asked not to go anywhere near that deadline when submitting RFPs. | 11:47 |
| lkcl- | i appreciate that this is "not a satisfactory answer" but i am now in a "reduced bandwidth" situation | 11:48 |
| *** ghostmansd[m] <ghostmansd[m]!~ghostmans@broadband-188-32-220-156.ip.moscow.rt.ru> has quit IRC | 11:48 | |
| *** ghostmansd[m] <ghostmansd[m]!~ghostmans@176.59.170.11> has joined #libre-soc | 11:49 | |
| *** ghostmansd <ghostmansd!~ghostmans@176.59.170.11> has joined #libre-soc | 11:56 | |
| ghostmansd | lkcl, this is not satisfactionary at all. I'll drop dataclass from Instruction, and will inherit it from dict and tune appropriately. However, I will keep dataclasses for all stuff associated with CSVs. | 11:58 |
| *** ghostmansd <ghostmansd!~ghostmans@176.59.170.11> has left #libre-soc | 12:01 | |
| *** ghostmansd <ghostmansd!~ghostmans@176.59.170.11> has joined #libre-soc | 12:01 | |
| ghostmansd | lkcl, this is not satisfactionary at all. I'll drop dataclass from Instruction, and will inherit it from dict and tune appropriately. However, I will keep dataclasses for all stuff associated with CSVs. | 12:01 |
| ghostmansd | This is only acceptable due to time constraints and because of our personal relations: this discussion isn't worth to have a conflict. Next time, I will need a valid technical argument, it's really rude and impolite to lead the discussion in this way. | 12:02 |
| ghostmansd | I literally spent several days to debug, analyze and refactor what we have. I put a validation for all fields there, caught several erros in fields, and did a hard work overall,, so at least some technical argument would be appropriate. | 12:04 |
| *** markos <markos!~Konstanti@62.74.116.34> has joined #libre-soc | 12:05 | |
| *** ghostmansd[m] <ghostmansd[m]!~ghostmans@176.59.170.11> has quit IRC | 12:05 | |
| *** ghostmansd <ghostmansd!~ghostmans@176.59.170.11> has quit IRC | 12:05 | |
| *** ghostmansd[m] <ghostmansd[m]!~ghostmans@176.59.57.177> has joined #libre-soc | 12:06 | |
| *** ghostmansd <ghostmansd!~ghostmans@176.59.57.177> has joined #libre-soc | 12:07 | |
| ghostmansd | So, TL;DR: Instruction class, which is used by other modules, will be dict-like; the rest will be kept intact, including CSVs entries. These will be internal after refactoring. | 12:07 |
| lkcl- | ghostmansd, this is appropriately assertive and well expressed. | 12:19 |
| lkcl- | please accept that i am still queasy and partly in shock which is colouring my words and making it difficult to express things | 12:25 |
| lkcl- | i do appreciate that this needs deep focus (and that's what you've done) | 12:25 |
| ghostmansd | lkcl, sure. Please have a rest today, we need you in a good mood and certainly healthy. | 12:32 |
| ghostmansd | Thank you for your time, especially considering the situation. | 12:32 |
| lkcl- | if an Instruction instance can be passed to a modified-PowerOp such that it can do a walk and create its needed Signals easily, i'm perfectly happy | 12:32 |
| lkcl- | so if the Instruction instance only has the fields needed (without needing to go "is this one i should know nothing about equal to None") then that is great | 12:33 |
| lkcl- | i'm going back to bed, before i fall over | 12:33 |
| lkcl- | hang on, wait, no i got it | 12:40 |
| lkcl- | you want Instruction to be "The Canonical Defined Source Of Everything" | 12:40 |
| lkcl- | ok i got it now | 12:40 |
| *** markos <markos!~Konstanti@62.74.116.34> has quit IRC | 12:41 | |
| lkcl- | then PowerOp would be told which bits to choose from that "database instance in nice format" (i.e. not a botch like a dict from a csv file) | 12:41 |
| lkcl- | in which case much as i detest dataclass, it's appropriate in this instance | 12:42 |
| lkcl- | so please don't waste time modifying Instruction | 12:42 |
| *** markos <markos!~Konstanti@62.74.116.34> has joined #libre-soc | 12:53 | |
| *** ghostmansd <ghostmansd!~ghostmans@176.59.57.177> has quit IRC | 13:26 | |
| *** tplaten <tplaten!~isengaara@55d4ae03.access.ecotel.net> has joined #libre-soc | 14:46 | |
| *** tplaten <tplaten!~isengaara@55d4ae03.access.ecotel.net> has quit IRC | 14:56 | |
| Veera[m] | Hi, Have any new work me!! | 14:58 |
| *** lxo <lxo!~lxo@gateway/tor-sasl/lxo> has quit IRC | 16:44 | |
| *** lxo <lxo!~lxo@gateway/tor-sasl/lxo> has joined #libre-soc | 16:45 | |
| *** alethkit <alethkit!23bd17ddc6@2604:bf00:561:2000::3ce> has joined #libre-soc | 18:10 | |
| *** ghostmansd[m] <ghostmansd[m]!~ghostmans@176.59.57.177> has quit IRC | 18:24 | |
| *** ghostmansd <ghostmansd!~ghostmans@broadband-188-32-220-156.ip.moscow.rt.ru> has joined #libre-soc | 18:52 | |
| ghostmansd | Yes, exactly. This was just a nicer dict from SVP64 record. However, it's too late. :-) | 19:22 |
| ghostmansd | Actually, I've took a step back and re-considered our discussion, and, even though the original approach would fit, I thought that you're right, too. | 19:22 |
| ghostmansd | So I encapsulated the stuff not intended to be visible on Instruction level: all these CSVs dataclasses are now hidden, and one can use getattr/__getitem__ in order to operate with it as with usual dictionary.\ | 19:23 |
| ghostmansd | In fact, I even thought subclassing the dict, but in the end decided to keep the thing immutable from API point of view. | 19:24 |
| ghostmansd | I'd have to override too many methods in dict(), too. | 19:24 |
| ghostmansd | https://git.libre-soc.org/?p=openpower-isa.git;a=blob;f=src/openpower/decoder/power_insn.py;h=e13d1c267432fa02f9d6893ba85a5d042c2c7715;hb=3256cc5c9fa4a003655bd3aba06c32a6a5f4b237#l315 | 19:26 |
| *** lxo <lxo!~lxo@gateway/tor-sasl/lxo> has quit IRC | 19:27 | |
| ghostmansd | Also, programmerjake, thank you for the cached_property wrapper! I wanted to use that, but noticed the Python version and surrendered. Also thanks for tip on caching iterators (basically they're not needed there, I simply decided to return tuples). | 19:27 |
| *** lxo <lxo!~lxo@gateway/tor-sasl/lxo> has joined #libre-soc | 19:29 | |
| ghostmansd | The reason why I ultimately chose to refactor it is, why should anyone know the _internal_ organization of our repository? All these links to PPCRecord and SVP64Record, why should anyone know how the stuff works _internally_? | 19:33 |
| ghostmansd | Nobody should give a fuck at all that these were different CSV entries upon using the class. | 19:34 |
| *** ghostmansd <ghostmansd!~ghostmans@broadband-188-32-220-156.ip.moscow.rt.ru> has left #libre-soc | 20:19 | |
| *** ghostmansd <ghostmansd!~ghostmans@broadband-188-32-220-156.ip.moscow.rt.ru> has joined #libre-soc | 20:19 | |
| *** ghostmansd[m] <ghostmansd[m]!~ghostmans@176.59.57.177> has joined #libre-soc | 20:22 | |
| *** ghostmansd[m] <ghostmansd[m]!~ghostmans@176.59.57.177> has quit IRC | 21:07 | |
| *** ghostmansd[m] <ghostmansd[m]!~ghostmans@broadband-188-32-220-156.ip.moscow.rt.ru> has joined #libre-soc | 21:08 | |
Generated by irclog2html.py 2.17.1 by Marius Gedminas - find it at https://mg.pov.lt/irclog2html/!