libera/#sicl - IRC Chatlog
Search
8:36:45
Duuqnd
Here's my implementation for a random number generator system for SICL: https://gist.github.com/johnlorentzson/f6c2389b9896c13ea3d9c23f95527070
8:38:20
Duuqnd
It's all in one file because it doesn't depend on other SICL modules and I just wanted to prototype it easily
8:39:08
Duuqnd
And the CL symbols used are shadowed but not exported because I don't know what the standard practice for that is in SICL and I just wanted to be able to test this in SBCL without issues.
8:40:16
Duuqnd
There's still a few things I'm not so sure about here but I'd like some feedback before I begin making a pull request
8:48:20
moon-child
Duuqnd: you do not need to ':reader random-bits' for mt19937-random; it will be inherited from random-state
8:51:56
moon-child
as a point of style, I think (let ((y ...)) (setf y) (setf y) (setf y)...) would do better to shadow than mutate. (let* so it doesn't end up deeply nested)
8:56:48
Duuqnd
I mainly did the three setf because I was more or less following the pseudo-code from Wikipedia and SBCL does the same but yeah, let* does make more sense.
8:59:25
Duuqnd
Alright, I've made the changes: https://gist.github.com/johnlorentzson/ed0ed853732b5c7a84e3df5319ecbdca
9:00:21
moon-child
ah, two more things. (loop for i... for xi-1 = (aref). Since the array in question is full of integers, xi-1 will never be NIL; but in principle if it became nil, it would halt the loop, so using for there can subvert expectations. So I would prefer _as_ xi-1 = ...
9:01:06
moon-child
and, you allow state-array to be set by the user (where it could have arbitrary length), but elsewhere assume its length is +mt19937-size+
9:01:16
moon-child
I am sure others have feedback as well. But make a PR! This is all very nitpicky stuff
9:02:54
Duuqnd
Alright, sounds good. Btw, should the CL symbols (random, make-random-state, etc) be exported by the sicl-random package?
9:08:30
beach
The intrinsic package would :USE the CL package, and export symbols that are not CL symbols but that can still be useful to client code. It can optionally export the CL symbols as well.
9:09:50
Duuqnd
What's a good example of this in use? I think you mentioned the sequence systems before?
9:11:54
beach
Yes, the sequence system was designed to work in any Common Lisp implementation as a separate package, and in SICL as the native implementation of the sequence functions.
9:14:03
Duuqnd
The only difference between packages.lisp and packages-intrinsic.lisp in Code/Sequence seems to be that one does :use #:closer-common-lisp and the other uses #:common-lisp (and imports from a few other things) so I'm not sure I'm understanding this.
9:16:06
Duuqnd
I didn't see any other differences between them though so I'm not sure what I'm supposed to do in this case. I think I'll just use one package and system when submitting the pull request and we can fix it then before it's merged.
9:42:23
Duuqnd
I noticed that RANDOM's handling of non-positive numbers wasn't correct so I had to fix that, but I've send the pull request now.
9:55:48
beach
Duuqnd: I very much like this way of organizing a system that has a single main package, like in this case a Common Lisp implementation with the CL package. There would be one package that has only the exported symbols in it. No code is written with (in-package #:cl) at the top. Instead, individual modules would each have a package the :USEs the CL package.
9:56:45
beach
I learned this technique from the CLIM specification. It is strange at first, because other programming languages would work the other way around.
10:00:14
beach
But since we never write any code with (in-package #:cl), we never pollute the host package.
10:04:58
jackdaniel
"when all lisp vendors implement x3j13 common lisp, the clim-lisp package could be eliminated" - but clim have assumptions about gray streams not being part of x3j13
10:06:57
jackdaniel
"A CLIM implementation might define a clim-internals package that uses each of the above packages" I gather that you have referred this - it is slightly different than sicl in my understanding
10:07:59
jackdaniel
in CLIM it is (define-package #:clim-internals (:use #:clim #:clim-lisp #:clim-extensions #:other-systems) …) and in a single package multiple "exported" protocols are implemented
10:08:28
jackdaniel
in SICL it is (defpackage #:random (:use #:cl)) (defpackage #:sequences (:use #:cl)) etc
10:09:42
jackdaniel
so I think that while the idea has the same roots it is noticeably different - CLIM is "all for one" and SICL is "one for all"; I'm not sure how to phrase it
10:09:58
beach
I should have said "SICL DEFINES many more packages than CLIM, but the idea is the same"
10:10:54
beach
The basic idea is the same. The main package CL or CLIM, contains only the specified symbols and they are all exported.
10:13:06
jackdaniel
yes, this part of the idea is the same -- what I'm trying to emphasize is the implementation strategy - whether protocol symbols are implemented in a /single/ package or in /many/ packages; but perhaps it doesn't matter much (especially that McCLIM has also backend-specific packages too). sorry for the noise
10:14:17
beach
No worries. You are right, of course. I think what CLIM does reflects a time when the package system was underused (for reasons I don't quite understand).
10:15:10
jackdaniel
I believe that using a single package for the implementation has benefits from the maintenance perspective - when working on systems split into multiple modules I often find myself shaving a yak when a symbol migrates from one package to another
10:15:31
jackdaniel
that is not a problem if multiple packages are only presented as an external interface and the implementation is in a single package
10:17:12
beach
If a symbol migrates, that would be evidence that the modules were not very well thought out in the first place.
10:18:05
jackdaniel
sure, but it is hard to foresee some future problems. and splitting a single package into multiple packages is also prone to issues - that is a case i.e for drei where were quite a few "dangling" symbols
10:18:55
beach
On the other hand, a single implementation package is subject to the use of the same symbol for different purposes in different "modules".
10:18:59
jackdaniel
but that's quite subjective and depends on the project; I'm just saying that I had a few headaches due to too many packages
10:20:12
jackdaniel
sure, I'm not saying that packages are not necessary or doesn't have use; I'm trying to point out that making them too fine grained has its drawbacks
10:22:19
beach
I guess I don't see the problem you describe so much because it is a tiny part of much of my daily work, which is to reorganize or rewrite things due to design decisions that seemed reasonable at the time, but that turned out to be bad when more information was available.
10:23:05
beach
I would avoid such activities if I knew a way to do it, but I don't believe much in the waterfall model, so this is how I do it instead.
11:55:25
Duuqnd
I just noticed that the random-state type is also defined in sicl-arithmetic. Is that going to be a problem?
12:13:27
beach
I had to reshuffle some more stuff because both Incless and ctype need the random-state definition, which is probably why I had it in arithmetic.
12:24:56
beach
If I attempt to load the sicl-random-intrinsic system early, it defines the function RANDOM that was previously imported from the host. And that would be fine, except that RANDOM is called by SICL MAKE-INSTANCE in order to allocate a hash code for new standard instances.
12:25:43
beach
So then the variable *random-state* is not yet defined. It is defined at the end of the file and instances are created as a result of loading preceding code.
12:34:27
beach
Heh, I see the problem. Even if I assign to *RANDOM-STATE* early, the initialization of that variable creates a standard instance, which then calls RANDOM.
12:35:37
Duuqnd
So basically we need to have randomness available before a RANDOM-STATE can be created?
12:38:46
beach
Maybe define RANDOM last in the file so that everything is in place before the host RANDOM is replaced.
12:45:31
beach
OK, so there is no order that will work here. I think I just need to define the RANDOM-STATE class twice. The problem is that you call MAKE-ARRAY at compile time, and MAKE-ARRAY requires SUBTYPEP which accesses the class RANDOM-STATE.
12:53:15
beach
So by loading the class definition before we load Incless and ctype, we will satisfy the methods of those systems, and we will still have the host RANDOM function working. That allows us to load the SICL-RANDOM-INTRINSIC system at some later point.
12:55:50
Duuqnd
I'm sorta struggling to remember what these other systems are. I remember ctype is the type system but I'm not sure what Incless is.
13:00:36
beach
I still can't load your system, but I think that's a problem in MAKE-ARRAY. I don't think the element-type (unsigned-byte 32) has been tested before, so it is probably a bug in (SETF ROW-MAJOR-AREF). So that's good that your system exercised this code.
13:04:57
Duuqnd
By the way, I was considering adding another RANDOM-STATE subclass that uses the operating system's RNG source which might be desirable sometimes. Would I need to do anything special regarding file I/O and OS-dependent code?
13:05:45
Duuqnd
Actually, now that I think about it, it might be impossible to make that conform to the standard since the OS's RNG state can't exactly be printed and read like the standard wants.
13:12:16
beach
Well, none of the OS interface code is implemented yet, so you will get a lot of warnings for undefined functions.
13:12:52
beach
And the OS interface won't be working in the host, so it can't be used during bootstrapping.
13:13:58
Duuqnd
Btw, what's the state of sicl-filename? It's basically empty and I'm wondering what kind of work would need to be done on it.
13:17:50
beach
I believe there is an external library for it. But ebrasca has consistently refused to accept the SICL coding conventions, so I have not been eager to look at the result.
13:20:35
beach
I can confirm that (make-array 10 :element-type '(unsigned-byte 32)) does not work at the REPL.
13:29:31
beach
Found the problem. The HIR evaluator, when evaluating the ASET-INSTRUCTION, does not take the element type into account.
13:42:23
Duuqnd
I'm glad to see that me not wanting to understand the 64-bit version of the Mersenne Twister helped find a bug
13:45:03
beach
Sure. But it's kind of not a bug. At bootstrapping time we make no attempt to get the full language to work, and we did not expect to have to deal with SICL arrays of this type during bootstrapping. But, as we see the need for more functionality to work at bootstrapping time, we obviously add it.
13:45:52
jackdaniel
isn't this similar to a strategy where the implementation is built on top of a language subset?
13:46:51
beach
Not really. At bootstrapping time we expect mostly definitions, not for target code to be evaluated.
13:47:55
beach
Now, of course, target CLOS code does get executed, because the best way to describe the graph of classes and generic functions is to execute the code that creates it.
13:49:44
beach
So we handle CHARACTER and (UNSIGNED-BYTE 8) at the moment. I'll just have to add (UNSIGNED-BYTE 32).
13:53:02
beach
Since we already handle CHARACTER, and characters are stored in 32-bits, adding a clause for (UNSIGNED-BYTE 32) should be straightforward.
13:55:06
beach
One down. Now I need get-universal-time at bootstrapping time, but that should be possible to import from the host.
13:57:09
Duuqnd
Would it be better to use a constant for the initial seed and then re-seed it after bootstrapping?
13:57:48
beach
jackdaniel: In fact it's amazing that we are able to execute target code at bootstrapping time at all. I think SBCL merely cross compiles on the host system, and then uses the resulting FASL files to patch together an executable. The paper by Krystof indicates that this part of SBCL bootstrapping is not idea. I forget the exact wording.
13:59:18
beach
Duuqnd: It should be fine as it is. These problems are normal bootstrapping problems when modules have cyclic dependencies. They just have to be resolved somehow, and this one is easy since the host GET-UNIVERSAL-TIME will work just fine during bootstrapping.
14:02:31
beach
Let me emphasize what I just said. The amount of target code that we are able to execute at bootstrapping time is just amazing to me.
14:05:18
beach
I suppose they must arrange for load-time code to be executed at start-up time or something, perhaps for a "cold" system.
14:11:19
beach
"... this 'cold init' phase is probably the most fragile portion of the SBCL build currently, and is also the hardest to debug..."
14:22:54
beach
Krystof's phrase continues "(because if it goes wrong, there will be no helpful Lisp debugger)."
14:26:31
beach
I should re-read the paper now that I understand more about what the issues are. Last time I read it, there were some aspects that were a bit mysterious to me.
15:13:26
pjb
Duuqnd: L147: this leaks an implementation detail. It would be better to hide it in a function; something like: (format stream "#.(sicl-random:make-random-state ~D ~D ~S)" #|version=|#1 (index object) (state-array object))