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

markovchain accepts sparse transition matrix #223

Merged
merged 6 commits into from
Oct 7, 2024

Conversation

reikoch
Copy link
Contributor

@reikoch reikoch commented Sep 21, 2024

Instead of a matrix from base R also matrices from package Matrix are now accepted for initialisation and coercion. A simple test has been added to the package.

As an aside - I took the liberty to fix the error message about non-square transition matrices. This should fix issue #222

@spedygiorgio
Copy link
Owner

@reikoch thank you really very much for your contribution. I will check it in deatils in the next days

@spedygiorgio
Copy link
Owner

@reikoch hello. May you support me in fixing R-CMD-check on ubuntu?

@reikoch
Copy link
Contributor Author

reikoch commented Oct 5, 2024

Certo Giorgio!

Ubuntu 22.04.5, with gcc 11.4.0, seems to have a problem with R-develop of 2024-10-01 r87205 - but only this very combination. R CMD check bails out when trying to install markovchain 0.10.0.9006, the error is detailed in 00install.out:

** byte-compile and prepare package for lazy loading
Creating a new generic function for ‘sort’ in package ‘markovchain’
** help
*** installing help indices
** building package indices
** installing vignettes
** testing if installed package can be loaded from temporary location
Error: package or namespace load failed for ‘markovchain’ in dyn.load(file, DLLpath = DLLpath, ...):
 unable to load shared object '/home/runner/work/markovchain/markovchain/check/markovchain.Rcheck/00LOCK-markovchain/00new/markovchain/libs/markovchain.so':
  /home/runner/work/markovchain/markovchain/check/markovchain.Rcheck/00LOCK-markovchain/00new/markovchain/libs/markovchain.so: undefined symbol: _Z14sortByDimNamesIN4Rcpp6MatrixILi14ENS0_15PreserveStorageEEEET_S4_
Error: loading failed
Execution halted
ERROR: loading failed

Right now I have no access to amd64 architecture computers. Using docker on my M2 Mac the package just checks fine for R-develop r87206 on Debian trixie with gcc 14.2.0.

Should we just wait for an upgrade of the ubuntu github runner?

Best regards
Reinhold

@reikoch
Copy link
Contributor Author

reikoch commented Oct 6, 2024

It seem not to be caused by c++. After some rearranging function declarations out of helper.h it is still only ubuntu on r-devel that fails - with a different symbol undefined:

** byte-compile and prepare package for lazy loading
Creating a new generic function for ‘sort’ in package ‘markovchain’
** help
*** installing help indices
** building package indices
** installing vignettes
** testing if installed package can be loaded from temporary location
Error: package or namespace load failed for ‘markovchain’ in dyn.load(file, DLLpath = DLLpath, ...):
 unable to load shared object '/home/runner/work/markovchain/markovchain/check/markovchain.Rcheck/00LOCK-markovchain/00new/markovchain/libs/markovchain.so':
  /home/runner/work/markovchain/markovchain/check/markovchain.Rcheck/00LOCK-markovchain/00new/markovchain/libs/markovchain.so: undefined symbol: _Z8_list2McN4Rcpp6VectorILi19ENS_15PreserveStorageEEEdb
Error: loading failed

Can it be that the github runner hits some limits, memory for instance?

@spedygiorgio
Copy link
Owner

@reikoch thank you very much for your efforts. I am going to merge the request tomorrow, but I would also like to ask you to add your name within the contributors. Also, might it be worthly to add a couple of sentence within the vignette to explain the functionalities you have added?

@codecov-commenter
Copy link

Welcome to Codecov 🎉

Once you merge this PR into your default branch, you're all set! Codecov will compare coverage reports and display results in all future pull requests.

Thanks for integrating Codecov - We've got you covered ☂️

@reikoch
Copy link
Contributor Author

reikoch commented Oct 7, 2024

It was just a little thing to open creation of markovchain objects for transistion matrices of class Matrix. I made a short reference in the main vignette and added myself to the contributors. Hope it is all fine!

@spedygiorgio spedygiorgio merged commit 191d093 into spedygiorgio:master Oct 7, 2024
@spedygiorgio
Copy link
Owner

@reikoch I merged the pull request and sent the file to be checked on wilbuilder. hope this works. Thanks a lot for your efforts.

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