Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

RFC: Refactor of random.jl, and stick into modules. #1213

Closed
wants to merge 4 commits into from
Closed

Conversation

ViralBShah
Copy link
Member

I have started refactoring random.jl, so that the librandom interfaces are in a separate module. I also want to add the capability of having multiple RNGs with different states, and generally rework the entire RNG interface.

The first bit of work is to simply keep the existing interfaces, but inside an RNG module, and refactor the librandom functions into a LibRandom module.

This pull request will serve to evolve the new RNG interfaces, which have been discussed in scattered places before.

Cc: @johnmyleswhite @dmbates

@ViralBShah
Copy link
Member Author

I currently get error during bootstrap: librandom not defined. I can't seem to figure out what I am missing.

@StefanKarpinski
Copy link
Member

You have to define librandom in Base at the top of the file like you did before.

@ViralBShah
Copy link
Member Author

I am doing everything that is done for FFTW, but I continue to get:

$ make
    JULIA usr/lib/julia/sys.ji
Warning: replacing module Base
librandom not defined
 in include at boot.jl:243
 in load at util.jl:230
 in load at util.jl:242
 in process_options at client.jl:175
 in _start at client.jl:227
at RNG.jl:98
 in load at util.jl:230
 in load at util.jl:242
 in process_options at client.jl:175
 in _start at client.jl:227
at /Users/viral/julia/base/sysimg.jl:115
 in load at util.jl:253
 in process_options at client.jl:175
 in _start at client.jl:227
make[1]: *** [/Users/viral/julia/usr/lib/julia/sys.ji] Error 1
make: *** [release] Error 2

@ViralBShah
Copy link
Member Author

@JeffBezanson Do the library names in start_image.jl have to continue to live in Base, or can we move them into the modules? For example, no user code except RNG.jl should really need access to the librandom dynamic library handle, and the contents of the LibRandom module. Only DSP requires access to the FFTW module and the libfftw handle.

Is it possible to hide all of this inside the modules, and not even expose in Base? I don't know what the impact would be on creation of sys.ji.

@ViralBShah
Copy link
Member Author

I can't quite figure out how to reproduce it, but after adding librandom to RNG.jl it builds sys.ji fine sometimes, but most of the times, while building I get:

 $ make
    JULIA usr/lib/julia/sys.ji
error during init:
could not show value of type LoadError
make[1]: *** [/Users/viral/julia/usr/lib/julia/sys.ji] Error 1
make: *** [release] Error 2

@StefanKarpinski
Copy link
Member

I've spent a bunch of time trying to debug this, but I can't figure out what's messed up. You've changed a lot of stuff here, which ends up being impossible to debug when bootstrapping because the process is so brittle and tricky to debug. I suggest going back and making incremental changes, each of which preserves the system as a working whole.

Also, since we consistently name files in lowercase, it would be nice to stick to that until such a time as we decide to capitalize file names and do so across the board.

@JeffBezanson
Copy link
Member

start_image and the current way of using libraries is going to go away.

@StefanKarpinski
Copy link
Member

That will be great but until then, this can still be refactored.

@ViralBShah
Copy link
Member Author

Thank GOD.

-viral

On 27-Aug-2012, at 11:37 PM, Jeff Bezanson [email protected] wrote:

start_image and the current way of using libraries is going to go away.


Reply to this email directly or view it on GitHub.

@ViralBShah
Copy link
Member Author

When is start_image going away? I'd rather just wait till then.

@ViralBShah
Copy link
Member Author

The changes in here are really straightforward. I factored out the librandom calls into a separate module, and put the rest of the rng stuff in its own module. Then all that remains is the dlopen stuff. Two possible troublesome things related to bootstrapping are the compile_hints and initialization in client.jl.

Can we clearly separate such modules out of the bootstrapping process?

@StefanKarpinski
Copy link
Member

That's a lot of stuff to change during bootstrap. Every time I've made a bunch of changes to bootstrap code, I end up doing binary search on the changes to find out what the hell broke. It might be possible to entirely extract RNG stuff from base, honestly. I'm not sure where in Base you would need random numbers.

@ViralBShah
Copy link
Member Author

I just left it there just because it was there before. We should move as many libraries out of Base as possible. There is no real reason for them to be in Base.

One thing that RNG needs is initialisation on loading of the module. Is that possible currently?

Of course, I would be annoyed if I had to do import RNG or some such thing every time I start julia. That is just a case of loading a bunch of modules apart from Base, by default.

-viral

On 28-Aug-2012, at 8:59 AM, Stefan Karpinski wrote:

That's a lot of stuff to change during bootstrap. Every time I've made a bunch of changes to bootstrap code, I end up doing binary search on the changes to find out what the hell broke. It might be possible to entirely extract RNG stuff from base, honestly. I'm not sure where in Base you would need random numbers.


Reply to this email directly or view it on GitHub.

@ViralBShah ViralBShah closed this Sep 7, 2012
KristofferC added a commit that referenced this pull request Feb 25, 2025
)

Stdlib: LinearAlgebra
URL: https://github.com/JuliaLang/LinearAlgebra.jl.git
Stdlib branch: master
Julia branch: master
Old commit: e7da19f
New commit: f781708
Julia version: 1.13.0-DEV
LinearAlgebra version: 1.12.0(Does not match)
Bump invoked by: @KristofferC
Powered by:
[BumpStdlibs.jl](https://github.com/JuliaLang/BumpStdlibs.jl)

Diff:
JuliaLang/LinearAlgebra.jl@e7da19f...f781708

```
$ git log --oneline e7da19f..f781708
f781708 use the custom sysimage when running documentation (and doctests) as well (#1226)
8fdbfd5 Add `getindex` for `SymTridiagonal` using a `BandIndex` (#1223)
91b8845 Use `BandIndex` directly in `diagzero` call in `getindex` (#1222)
ef7ef3a Restrict triangular type aliases to `AbstractMatrix`es (#1220)
af7a9ac Use `BandIndex` directly in `diagzero` call in `getindex` for banded matrices
579b5f7 Specialize Diagonal * Adjoint (#1207)
5cf41c4 Indirection in matrix multiplication to avoid ambiguities (#1210)
0a9c164 Remove specialized `issymmetric`/`ishermitian` for `Diagonal{<:Number}` (#1213)
ff5648a Make unitary matrices in `svd`/`eigen` of `Diagonal` be unitless (#1155)
e096a03 Don't mutate arrays in symmetric trig functions (#1206)
c234bed Loop over `diagind` in `diag` for banded matrices (#1211)
57785c7 More resizing for truncating return values from LAPACK (#1190)
b464203 Materialize adjoint in mul with `HermOrSym` (#1191)
16d9d61 Restrict Diagonal sqrt branch to positive diag (#1203)
baa48b7 Verbose `showerror` for `SingularException` (#1204)
e0b59a7 Remove `LinearAlgebra` qualifications in `cholesky.jl` (#1209)
95d009b Remove `LinearAlgebra` qualifications in `cholesky.jl`
c550974 change to pivot
ed35a37 Add fast path in generic matmul (#1202)
8c7fe68 Detailed `showerror` for `SingularException`
2a1696a Explicitly declare type constructor imports (#1196)
101f766 Added note to BLAS.[g|s]et_num_threads about Apple Accelerate not supporting it  (#1195)
5aca26f Simplify `getproperty` for `Cholesky*` (#1197)
924dda4 remove copy-allocation on accessing `cholesky` factors (`.L`, `.U`) (#1186)
6f02532 Use `BLAS.trsm!` instead of `LAPACK.trtrs!` in left-triangular solves (#1194)
```

Co-authored-by: KristofferC <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants