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

v4.4.1 #162

Merged
merged 75 commits into from
Oct 21, 2023
Merged

v4.4.1 #162

merged 75 commits into from
Oct 21, 2023

Conversation

Neves-P
Copy link
Collaborator

@Neves-P Neves-P commented Oct 10, 2023

DAISIE 4.4.1

@Neves-P Neves-P self-assigned this Oct 10, 2023
@Neves-P
Copy link
Collaborator Author

Neves-P commented Oct 10, 2023

@joshwlambert @rsetienne , could you please take a quick look at the NEWS.md and see if I'm forgetting anything? The text is the same as the first comment of this PR.

@joshwlambert
Copy link
Collaborator

It would be useful to link the pkgdown website in the About section on the DAISIE github repo.

Copy link
Collaborator

@joshwlambert joshwlambert left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've reviewed the changes and everything looks good to me.

There is nothing missing from the NEWS file that I could see other than extremely minor changes, e.g. explicit namespacing of tests.

I would suggest submitting to CRAN in order to easily use the new changes when running models locally and on the cluster.

@joshwlambert
Copy link
Collaborator

As far as I can see the test that has the numerical discrepancy issue has now been skipped. Would it be possible to log an issue stating which test (file name and line number) is causing the issue? This can then be tackled in a follow-up PR.

@joshwlambert
Copy link
Collaborator

@rsetienne @Neves-P It would be great to get these changes merged and a new version of DAISIE sent to CRAN to easily install the package across different platforms.

@Neves-P
Copy link
Collaborator Author

Neves-P commented Oct 20, 2023

This looks good to me, I just added a tolerance again and unskipped the test after having checked with Hanno and @rsetienne that these numerical differences are to be expected. @rsetienne , as soon as GHA are done, could you please merge or should I go ahead (there is already one review, so I can merge it).

@codecov-commenter
Copy link

Codecov Report

Merging #162 (35cad4c) into master (ebf13e6) will decrease coverage by 1.43%.
Report is 1 commits behind head on master.
The diff coverage is 73.80%.

❗ Current head 35cad4c differs from pull request most recent head b0d29f2. Consider uploading reports for the commit b0d29f2 to get more accurate results

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

@@            Coverage Diff             @@
##           master     #162      +/-   ##
==========================================
- Coverage   68.54%   67.11%   -1.43%     
==========================================
  Files          78       79       +1     
  Lines        7964     8123     +159     
==========================================
- Hits         5459     5452       -7     
- Misses       2505     2671     +166     
Files Coverage Δ
R/DAISIE_ML1.R 82.50% <100.00%> (-0.77%) ⬇️
R/DAISIE_ML_CS.R 41.26% <ø> (ø)
R/DAISIE_SR_loglik_CS.R 29.55% <100.00%> (-55.37%) ⬇️
R/DAISIE_loglik_CS.R 81.27% <100.00%> (-0.86%) ⬇️
R/DAISIE_utils.R 55.31% <ø> (ø)
R/create_pars.R 71.15% <ø> (ø)
R/DAISIE_loglik_integrate.R 88.41% <75.00%> (-0.48%) ⬇️
R/DAISIE_loglik_CS_shift.R 71.71% <71.71%> (ø)

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@rsetienne rsetienne merged commit 2051f8e into master Oct 21, 2023
11 checks passed
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.

4 participants