RGF-team / rgf

Home repository for the Regularized Greedy Forest (RGF) library. It includes original implementation from the paper and multithreaded one written in C++, along with various language-specific wrappers.
378 stars 58 forks source link

update to CRAN version 1.0.7 #340

Closed mlampros closed 3 years ago

mlampros commented 3 years ago

This pull request is for the update of the RGF R package to CRAN version 1.0.7. As discussed in the comment of the pull request 338 I've modified the code because there were 2 NOTES for the Windows version that had to be fixed. As for the additional issue of the new M1mac chip-tests that was discussed in the comment of the pull request 338 we have to keep an eye on the check results page of the R package.

mlampros commented 3 years ago

It gives an error or R ubuntu-latest

* checking PDF version of manual ... WARNING
LaTeX errors when creating PDF version.
This typically indicates Rd problems.
LaTeX errors found:
Undefined control sequence.
....

I re-run the tests but it persists. I don't think that it's related with the code.

In the second run of the tests it gives a NOTE on mac OS because one example ran for more than 5 seconds.

Is there any way to re-run only the R tests? It seems that the Re-run jobs button applies to all tests (both R and Python)

StrikerRUS commented 3 years ago

@mlampros Thank you very much for your hard work!

I re-run the tests but it persists. I don't think that it's related with the code.

We are seeing some LaTeX-related errors in LightGBM right now: https://github.com/microsoft/LightGBM/pull/3997#issuecomment-801471537 too. Let's re-run tests tomorrow and see whether we need to dig deeper into the problem or it is temporary just due to some servers' problems.

Is there any way to re-run only the R tests?

Unfortunately, it is impossible in GitHub Actions to re-run a separate job.

Only one frustrating thing that I noticed during more than 6 months of usage is that you cannot re-run a separate job, you should rerun all of them. https://github.com/RGF-team/rgf/pull/328#issue-564559326

StrikerRUS commented 3 years ago

In the second run of the tests it gives a NOTE on mac OS because one example ran for more than 5 seconds.

Please increase the allowed time to run by overwriting _R_CHECK_EXAMPLE_TIMING_THRESHOLD_ environment variable. Refer to https://github.com/microsoft/LightGBM/issues/4049#issuecomment-793412254 and https://github.com/microsoft/LightGBM/pull/4055/files.

mlampros commented 3 years ago

I am out of ideas for this error case on ubuntu-latest. I tried various modifications in the 'r_tests.sh' file but none of these work.

As a reference I'll add the following links that I stumbled upon during my search:

StrikerRUS commented 3 years ago

@mlampros I've just re-run all CI jobs in this PR and at the same time created a dummy PR from the master branch. That PR has been built successfully. So now we can be sure that the reason of failing CI is in the changes from this PR. I'll try to investigate breaking changes this weekend.

mlampros commented 3 years ago

So, you mean that my changes in the examples section of the package is the main reason for the error (actually the .Rd documentation error). But this error would have appeared also during the submission to CRAN, which is not the case.

StrikerRUS commented 3 years ago

So, you mean that my changes in the examples section of the package is the main reason for the error (actually the .Rd documentation error).

Yeah, it seems to be so.

But this error would have appeared also during the submission to CRAN, which is not the case.

I guess the difference in the environments of our CI and CRAN (software versions or incomplete installation, for example) led to such results.

StrikerRUS commented 3 years ago

@mlampros Hey!

I'll try to investigate breaking changes this weekend.

Please merge #342 into this PR to make it pass all CI checks. I left some comments there explaining fixes.

StrikerRUS commented 3 years ago

@mlampros Seems that I've managed to make our CI checks compatible with roxygen2 > 7.0. Please take a look: #343. Sorry for the inconvenience!