-
Notifications
You must be signed in to change notification settings - Fork 41
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
Docs update I #101
Docs update I #101
Conversation
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## master #101 +/- ##
==========================================
+ Coverage 81.34% 81.37% +0.02%
==========================================
Files 42 42
Lines 5549 5561 +12
==========================================
+ Hits 4514 4525 +11
- Misses 1035 1036 +1 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for this, doc updates are always very welcome.
randuniform([::Type{T}=Float64], dims::Dims{N}) -> Array{T,N} | ||
|
||
Create an array of size `dims` with random entries uniformly distributed in the allowed | ||
values of `T`. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
uniform is typically understood to be the uniform distribution on [0,1]. It is definitely not uniform over all values of T
(which would not even be well defined).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah I got confused by not reading the docstring of rand
far enough:
help?> rand
search: rand randn transcode macroexpand @macroexpand @macroexpand1
rand([rng=default_rng()], [S], [dims...])
Pick a random element or array of random elements from the set of values
specified by S; S can be
• an indexable collection (for example 1:9 or ('x', "y", :z)),
• an AbstractDict or AbstractSet object,
• a string (considered as a collection of characters), or
• a type: the set of values to pick from is then equivalent to
typemin(S):typemax(S) for integers (this is not applicable to
BigInt), to [0, 1) for floating point numbers and to [0, 1)+i[0,
1) for complex floating point numbers;
I am wondering though if this method really makes sense, as it is just a copy of rand
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe not, but this also applies to randnormal
which is just equal to randn
.
src/auxiliary/random.jl
Outdated
""" | ||
randnormal([::Type{T}=Float64], dims::Dims{N}) -> Array{T,N} | ||
|
||
Create an array of size `dims` with random entries normally distributed. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The normal distribution with mean zero and standard deviation one is typically called the standard normal distribution, so maybe we should try to get this in.
src/auxiliary/random.jl
Outdated
randisometry([::Type{T}=Float64], dims::Dims{2}) -> Array{T,2} | ||
randhaar([::Type{T}=Float64], dims::Dims{2}) -> Array{T,2} | ||
|
||
Create a random isometry of size `dims`. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess here you can mention the Haar measure or uniform measure over the compact manifold of isometric matrices.
src/fusiontrees/manipulations.jl
Outdated
-> <:AbstractDict{FusionTree{I,N-2*N₃}, <:Number} | ||
|
||
Perform a planar trace of the uncoupled indices of the fusion tree `f` at `q1` with those at | ||
`q2`, which are required to be pairwise neighbouring. The result is returned as a dictionary |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
pairwise neighbouring might be a bit confusing, as there are different ways in which traces of several indices can be planar:
It could for example be that q2[1] = q1[1]+1 and q2[2] = q1[2]+1, but also that q2[2] = q1[2]-3 , namely with q2[2] = q2[1] +1 = q1[1]+2 = q1[2] + 3
src/sectors/sectors.jl
Outdated
""" | ||
Trivial | ||
|
||
Singleton type to represent the trivial sector, i.e. the unit element of the group |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This can get a bit confusing. The trivial group has only a unit element, but Trivial()
typically (in our use case) refers to the only irrep of the trivial group, which is of course the trivial irrep and acts as the unit element in the category Rep[trivial group]. But I also don't immediately know the best way to phrase this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe it could also be mentioned that this is the unit object of the category Vect of ordinary vector spaces (not as the first sentence).
src/tensors/abstracttensor.jl
Outdated
""" | ||
numin(::Union{T,Type{T}}) where {T<:AbstractTensorMap} -> Int | ||
|
||
Return the number of input spaces of a tensor. This should be equivalent to `length(domain(t))`. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same remark about t
src/tensors/abstracttensor.jl
Outdated
""" | ||
numind(::Union{T,Type{T}}) where {T<:AbstractTensorMap} -> Int | ||
|
||
Return the total number of input and output spaces of a tensor. This should be equivalent to `numout(t) + numin(t)`. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same remark about t
src/tensors/abstracttensor.jl
Outdated
dim(t::AbstractTensorMap) -> Int | ||
|
||
The total number of free parameters of a tensor, discounting the entries that are | ||
fixed by symmetry. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it could be mentioned that this is the dimension of the hom space on which the tensormap is defined.
src/tensors/abstracttensor.jl
Outdated
""" | ||
domainind(::Union{T,Type{T}}) where {T<:AbstractTensorMap} -> Tuple{Int} | ||
|
||
Provide all indices of the domain of a tensor. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I find provide
a strange word here, in contrast to return
. Not sure why, could be subjective. For one of the other reason, it feels like Provide
requires the user to provide something.
src/tensors/abstracttensor.jl
Outdated
@doc """ | ||
fusiontrees(t::AbstractTensorMap) | ||
|
||
Return an iterator over all fusion trees of a tensor. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't recall if on other places I have called this "splitting-fusion-tree pairs".
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It seems like fusion-splitting tree pairs is mentioned in the documentation somewhere, as well as splitting-fusion tree pairs. I am completely fine with either, but I think the current naming is definitely confusing.
This is a first step in a rewrite of the documentation. The goal is to add docstrings to all exported methods and have a correctly structured lib and index part, before rethinking how to structure the manual part of the documentation.