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

reading neurons from neuronlistfh objects is very slow #402

Closed
jefferis opened this issue Jul 18, 2019 · 2 comments
Closed

reading neurons from neuronlistfh objects is very slow #402

jefferis opened this issue Jul 18, 2019 · 2 comments

Comments

@jefferis
Copy link
Collaborator

jefferis commented Jul 18, 2019

There is a new behaviour in the latest version of R which interacts very badly with a piece of code that loads neurons from neuronlistfh objects. Basically it forces garbage collection after reading every single neuron. The relevant code in "[[.neuronlistfh" was always crufty. I'm not sure if the external problem (in filehash package) is now fixed.

I think the definition of showConnections must have changed in R >=3.6.0

> showConnections
function (all = FALSE) 
{
    gc()
    set <- getAllConnections()
    if (!all) 
        set <- set[set > 2L]
    ans <- matrix("", length(set), 7L)
    for (i in seq_along(set)) ans[i, ] <- unlist(summary.connection(set[i]))
    rownames(ans) <- set
    colnames(ans) <- c("description", "class", "mode", "text", 
        "isopen", "can read", "can write")
    if (!all) 
        ans[ans[, 5L] == "opened", , drop = FALSE]
    else ans[, , drop = FALSE]
}
<bytecode: 0x7fe0f1afef58>
<environment: namespace:base>
@jefferis
Copy link
Collaborator Author

The original problem in filehash was here rdpeng/filehash#3. It looks like it was fixed 5 years ago! So depending on filehash >=2.3 should be fine.

@jefferis
Copy link
Collaborator Author

The problem in my code is here:

https://github.com/jefferis/nat/blob/f164b036ff755b187e81817f6a5e0498c4b3881e/R/neuronlistfh.R#L253-L276

switching from showConnections to getAllConnections seems to work fine. But I now see no need to do this at all given filehash fix.

jefferis added a commit that referenced this issue Jul 18, 2019
* this already makes a big difference
* progress on #402
jefferis added a commit that referenced this issue Aug 22, 2019
* closes #402
* depending on filehash >=2.3 ensures that the bug this code was trying to kludge over
  is fixed at source.
jefferis added a commit that referenced this issue Aug 22, 2019
* closes #402
* depending on filehash >=2.3 ensures that the bug this code was trying to kludge over
  is fixed at source.
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

No branches or pull requests

1 participant