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

Use (inv)logcdf in tests + update CI #12

Merged
merged 7 commits into from
Oct 28, 2021
Merged

Use (inv)logcdf in tests + update CI #12

merged 7 commits into from
Oct 28, 2021

Conversation

devmotion
Copy link
Member

Alternative to #11. Let's see if it is sufficient to perform computations in log space.

@davibarreira
Copy link
Member

If you don't want to sample from mu, another alternative is to have a larger value for the variance in order to guarantee that the sampled point is not too far way.

@@ -73,7 +73,7 @@ Random.seed!(100)
# compute OT plan
Copy link
Member

Choose a reason for hiding this comment

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

Perhaps try using ν = Normal(randn(), rand()+1) and the same for mu.

Copy link
Member Author

Choose a reason for hiding this comment

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

Sure, let's try it.

Copy link
Member Author

Choose a reason for hiding this comment

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

Seems to work 🎉

@devmotion devmotion changed the title Use (inv)logcdf in tests Use (inv)logcdf in tests + update CI Oct 28, 2021
@coveralls
Copy link

coveralls commented Oct 28, 2021

Pull Request Test Coverage Report for Build 1396553910

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+3.7%) to 96.835%

Totals Coverage Status
Change from base Build 1199181442: 3.7%
Covered Lines: 153
Relevant Lines: 158

💛 - Coveralls

@devmotion
Copy link
Member Author

Any additional comments or suggestions @davibarreira?

@davibarreira
Copy link
Member

Any additional comments or suggestions @davibarreira?

Yeah, I forgot to write in the review, but I think we should sample more than one point. Like:

x = randn(10)
γ.(x) \approx invlogcdf(ν, logcdf(μ, x))

@devmotion
Copy link
Member Author

Done.

@davibarreira
Copy link
Member

All good.

@devmotion devmotion merged commit e9efd4e into main Oct 28, 2021
@devmotion devmotion deleted the dw/logcdf branch October 28, 2021 21:55
@coveralls
Copy link

coveralls commented Sep 25, 2024

Pull Request Test Coverage Report for Build 1396494963

Warning: This coverage report may be inaccurate.

This pull request's base commit is no longer the HEAD commit of its target branch. This means it includes changes from outside the original pull request, including, potentially, unrelated coverage changes.

Details

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+3.7%) to 96.835%

Totals Coverage Status
Change from base Build 1199181442: 3.7%
Covered Lines: 153
Relevant Lines: 158

💛 - Coveralls

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