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 of SIMD computation in CRHMC walk #245

Closed
wants to merge 183 commits into from

Conversation

iakoviid
Copy link
Contributor

@iakoviid iakoviid commented Sep 12, 2022

Expanding on PR #239 we use the SIMD capabilities of PackedCSparse and present a vectorized CRHMC walk.
This way we can have speed up on medium to large datasets, depending on the systems architecture.
Changes from #239 :

  1. Changes in CRHMC walk instead of using n-dimensional vectors we use n * simd_len-dimensional matrices. In this way we do simd_len parallel CRHMC walks without spawning additional threads.
  2. Changes in testing and benchmarking files to test the behavior of the added code.


}

logconcave_sample<- function(P,distribution, n_samples ,n_burns){
Copy link
Collaborator

Choose a reason for hiding this comment

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

fix coding style

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

@@ -0,0 +1,61 @@
#ifndef COMMON_HPP
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can you give a more descriptive name to the file?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

@@ -0,0 +1,112 @@
%%MatrixMarket matrix coordinate real general
Copy link
Collaborator

Choose a reason for hiding this comment

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

What does MatrixMarket mean?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It is a popular sparse matrix file format: https://math.nist.gov/MatrixMarket/mmio-c.html

Copy link
Member

Choose a reason for hiding this comment

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

Great, could you please add this sentence as a comment?

@@ -16,6 +16,6 @@ For example: -DLP_SOLVE=/usr/lib/lpsolve/liblpsolve55.so

Copy link
Collaborator

Choose a reason for hiding this comment

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

Please add plots from the results from the various polytopes.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

Copy link
Collaborator

@papachristoumarios papachristoumarios left a comment

Choose a reason for hiding this comment

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

Impressive job! I have a few comments, mostly of aesthetical nature, but overall the PR looks good to me!

@vissarion
Copy link
Member

@iakoviid there are some issues still in CI tests (most probably new). There is one SEGFAULT with clang-12 and an error in R tests in windows (where some psrf is NaN). Could you please check?

iakoviid and others added 4 commits February 9, 2023 15:40
* Enable github actions to build examples. Avoid passing a polytope as a const reference.

* Fix ambiguous call to fix function by renaming volesti's diagnostic function. (GeomScale#263)

* Updating documentation (GeomScale#261)

Adding WSL and MKL build instructions.

* disable an R sampling test for windows

* Fix the warning message in R Mac's cran test (GeomScale#285)

* copy and replace lp_rlp.h

* remove re-initialization of eta

* update ubuntu version from 18 to 20 in R cran tests

* minor improvements (explanatory comments)

---------

Co-authored-by: Apostolos Chalkis <[email protected]>

* delete commented out code

* No-U-Turn Sampler in HMC (GeomScale#216)

* initialize nuts sampler class

* implement the burnin of nuts sampler

* add tests and resolve bugs

* implement e0 estimation in burnin of nuts

* optimize leapfrog

* integrate nuts into the R interface

* document random walk in sample_points.cpp in R interface

* fix burnin for the non-truncated case

* resolve bugs in hmc and nuts pipelines

* improve the preprocess in burin step of nuts

* split large lines in sample_points.cpp

* Add NUTS C++ example and update CMake (GeomScale#29)

* resolve PR comments

* fix minor bug

* fix compiler bug

* fix error in building the C++ examples

* resolve warnings in sample_points

* fix lpsolve cran warning

* fix cran warning on mac

* improve lpsolve cmake for cran check

* fix R warning in mac test

* remove lpsolve header

* resolve PR comments

---------

Co-authored-by: Marios Papachristou <[email protected]>
Co-authored-by: Apostolos Chalkis <[email protected]>

---------

Co-authored-by: Vissarion Fisikopoulos <[email protected]>
Co-authored-by: Soumya Tarafder <[email protected]>
Co-authored-by: Apostolos Chalkis <[email protected]>
Co-authored-by: Marios Papachristou <[email protected]>
@TolisChal
Copy link
Member

I'm closing this PR since it is included in #286

@TolisChal TolisChal closed this Nov 1, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants