search

refunders / refund

Regression with functional data
39 stars 23 forks source link

add mfpca.face #98

Closed ecui1 closed 2 years ago

ecui1 commented 2 years ago

We added the mfpca.face() function to refund, which implements the fast MFPCA method that we have been working on. We checked the package and it worked well on our laptops and passed R CMD check.

fabian-s commented 2 years ago

Thank you for the pull request, that's great.

Please add some unit tests for the new code that uses its basic functionality on some simple examples.

This also gives us a new dependency on simex, for using just a single function. Would it be possible to avoid simex::diag.block and use a straight kronecker product or Matrix::bdiag? We already depend on Matrix anyway, so either of those would be preferable.

ecui1 commented 2 years ago

Thanks for the review. We have updated the function and pushed it to the forked repo.

We have added some unit tests to 'test-mfpca.R' in the tests folder, and removed the dependency on simex package.

The new function works well, and the package passed R CMD check.

Please let us know if you have any other comments/questions.

fabian-s commented 2 years ago

I'll let @julia-wrobel take care of the rest ... ;)

julia-wrobel commented 2 years ago

Thanks for reviewing @fabian-s!