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

Potential bug with diag #216

Open
howardnewyork opened this issue Aug 16, 2018 · 5 comments
Open

Potential bug with diag #216

howardnewyork opened this issue Aug 16, 2018 · 5 comments
Milestone

Comments

@howardnewyork
Copy link

diag function does not seem to be working with greta arrays, e.g.:

N=100
xx=as_data(1)
dim(diag(xx,N,N))

produces
[1] 1 1

but should be
[1] 100 100

@goldingn
Copy link
Member

Yeah, diag() can't be used to create greta arrays yet, only to extract or replace elements. That's documented here, though it should error with a helpful message. Its also an annoying thing not to have and shouldn't be difficult to fix.

I think this is a duplicate of #108, but this has more info. So I'll close that one. Thanks!

@jeffreypullin
Copy link
Contributor

jeffreypullin commented Nov 26, 2018

I have a branch on my fork which fixes this. I insert:

# if the greta_array is like a vector then construct a greta_array matrix
if (length(dim[2]) == 1) {
  vec <- as.vector(get_node(x)$.value)
  out <- base::diag(vec, nrow, ncol)
  return(as.greta_array(out))
}

into the diag.greta_array method. I'm not sure if this is the best way to do this though.

@goldingn
Copy link
Member

goldingn commented Dec 6, 2018

Nice, thanks!

I'm hoping to cover all of the different ways of constructing a diagonal matrix with diag, which, from memory, also includes creating non-square diagonal matrices.

When I get to that I'll try to include your fork though. Could you please point me to it?

@jeffreypullin
Copy link
Contributor

My fork is Voltemand/greta/issue216 (I have now stopped naming my branches after issues as I forget what they are all about!) I basically insert the following code into diag.greta_array:

# try to construct a greta array if either nrow or ncol are specified
  if (!missing(nrow) | !missing(ncol)) {

      if (dim[2] != 1) {
          stop("'nrow' or 'ncol' cannot be specified when 'x' is not a vector",
               " like greta_array")

      } else {
          vec <- as.vector(get_node(x)$value())
          out <- diag(vec, nrow, ncol)
          return(as.greta_array(out))
      }
  }

I think (hope?) that the use of diag again (possibly it should be base::diag?) covers the cases you mentioned.

All the best!

@goldingn
Copy link
Member

goldingn commented Dec 6, 2018

Great, thanks! I'll write up some tests to cover all the documented behaviours and give this a go.

@njtierney njtierney added this to the 0.5.0 milestone Jul 2, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants