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

Update based on some recent developments #379

Merged
merged 46 commits into from
Feb 16, 2024
Merged

Update based on some recent developments #379

merged 46 commits into from
Feb 16, 2024

Conversation

osorensen
Copy link
Collaborator

No description provided.

* parallelization working

* updating tests

* styling

* updating test

* added info about parallelization
* added prior sampling

* updating news

* deleting submission file

* fixing the prior sampling

* styling

* added unit test for SMC starting from prior

* added test for sample_prior

* updating test after merging with TBB

* styling

* updating news
* fixing item name issue

* updating a unit test

* styling
* added handling of vector data #361

* styling
* updated set_priors function

* closes #370
* updated set_priors function

* closes #370

* fixed forgotten implementation
* updated documentation

* adding references section

* more documentation updates

* updated set_priors function

* closes #370

* implemented user-defined lag

* added lag, closes #369

* styling
@osorensen osorensen marked this pull request as draft February 16, 2024 12:24
@osorensen osorensen requested a review from wleoncio February 16, 2024 12:34
@osorensen osorensen marked this pull request as ready for review February 16, 2024 12:34
@osorensen
Copy link
Collaborator Author

@wleoncio the CodeFactor issues look like false positives to me, since the code it is referring to is in work-docs. Do you know how we can make CodeFactor ignore everything in that directory?

@wleoncio
Copy link
Member

wleoncio commented Feb 16, 2024

Sure, we could add something like this to the top level (no indent) of codecov.yml

ignore:
    - "work-docs"

@wleoncio
Copy link
Member

Strange. Maybe CF is not even reading the config file? 🤔

@wleoncio
Copy link
Member

wleoncio commented Feb 16, 2024

Just edited https://www.codefactor.io/repository/github/ocbe-uio/bayesmallows/ignore accordingly, maybe that'll do it.

bilde

@osorensen
Copy link
Collaborator Author

Could it be so that it checks the config file that is on the master branch??

@wleoncio
Copy link
Member

Could it be so that it checks the config file that is on the master branch??

I'm not even sure it's reading any file at all. On my other projects I don't even have a config file for CodeFactor 😅

@wleoncio
Copy link
Member

Anyway, we can address all code smells on a different PR, I'll review the code now.

NEWS.md Show resolved Hide resolved
Comment on lines 72 to 87
pi <- compute_posterior_intervals(mod2)
expect_equal(pi$parameter, "alpha")
expect_equal(pi$median, "2.744")
expect_equal(pi$hpdi, "[2.015,3.476]")
expect_equal(pi$central_interval, "[2.047,3.530]")
expect_equal(pi$median, "2.745")
expect_equal(pi$hpdi, "[2.009,3.517]")
expect_equal(pi$central_interval, "[2.016,3.519]")

mod3 <- update_mallows(
mod2,
new_data = setup_rank_data(potato_visual[10:12, ])
)

pi <- compute_posterior_intervals(mod3, decimals = 2)
expect_equal(pi$hpdi, "[2.18,3.75]")
expect_equal(pi$hpdi, "[2.23,3.74]")
pi <- compute_posterior_intervals(mod3, parameter = "rho")
expect_equal(pi$hpdi[[20]], "[1,5]")

Copy link
Member

Choose a reason for hiding this comment

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

This is alright, but for future-proofing, perhaps we could compare numbers with a high tolerance (like 0.1)?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I agree. I just have to figure out the random number generation stuff on my M1 mac. On all other platforms the results are reproduced, but M1 mac seems to work differently, even though I use Rcpp's random number generators which are designed to by platform independent. I don't know if it's the RNGs fault or if there are differences in floating point representations or something like that which causes the input to the functions for generating random numbers to differ. I think my next laptop will run Linux.

Copy link
Member

@wleoncio wleoncio left a comment

Choose a reason for hiding this comment

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

Good job!

@osorensen
Copy link
Collaborator Author

Thanks @wleoncio! I added #382 which is a simple fix.

@wleoncio
Copy link
Member

No problem, looks good. We'll fight CodeFactor later...

@osorensen osorensen merged commit 8850531 into master Feb 16, 2024
13 of 14 checks passed
@osorensen osorensen deleted the develop branch February 16, 2024 15:33
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.

2 participants