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

Fixed examples and added CI support for compiling examples #214

Closed
wants to merge 2 commits into from

Conversation

shahpratham
Copy link

This fixes #193.

This fixes CMakeLists.txt of vpolytope-volume and EnvelopeProblemSOS. All examples are now compiling successfully.

I have skipped compiling of optimization_spectrahedra in CI, as it required installation of the openblas, lapack, and arpack libraries. So, except that all files are being compiled in Github actions.

@codecov
Copy link

codecov bot commented Feb 28, 2022

Codecov Report

Merging #214 (329ff52) into develop (2e6572d) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff            @@
##           develop     #214   +/-   ##
========================================
  Coverage    55.24%   55.24%           
========================================
  Files           85       85           
  Lines         4802     4802           
  Branches      2108     2108           
========================================
  Hits          2653     2653           
  Misses         896      896           
  Partials      1253     1253           

@vissarion vissarion changed the title Fixed examples and added CI integeration for compiling CMakefiles Fixed examples and added CI integeration for compiling examples Feb 28, 2022
@vissarion vissarion changed the title Fixed examples and added CI integeration for compiling examples Fixed examples and added CI support for compiling examples Feb 28, 2022
@vissarion
Copy link
Member

vissarion commented Mar 3, 2022

Thanks for this PR. I am ok with merging. Although it does not resolve the issue #193 since optimization_spectrahedra examples are not included. I am OK with submitting another PR for optimization_spectrahedra or resolve it in the current PR.

@shahpratham
Copy link
Author

shahpratham commented Mar 3, 2022

resolve it in the current PR.

Okay, I have added those dependencies in the current PR.
Kindly review.

@vissarion
Copy link
Member

resolve it in the current PR.

Okay, I have added those dependencies in the current PR. Kindly review.

Thanks. However, right now only configurations (i.e. cmake) are run in CI. Please add make command in the scripts to actually build the examples.

@shahpratham
Copy link
Author

shahpratham commented Mar 3, 2022

Please add make command in the scripts to actually build the examples.

Actually I was able to run make command in almost all examples 2 weeks ago, but now it's not building. I'm sure it's not because of changes in made by me in CMakLists.txt as I even checked to build in the develop branch of voltesti, still not able to.

Examples which are failing to build are :- EnvelopeProblemSOS, hpolytope-volume, logconcave, multithread_sampling, optimization_spectrahedra, sampling-hpolytope-with-billiard-walks, vpolytope-volume.

Error screenshot of some of them:-
1

2

3

@vissarion
Copy link
Member

For the second one it seems that you are missing mkl. For the 1st and 3rd it seems that some changes in develop broke the examples. But the fix seems not that hard, passing the correct const/non const parameters to functions. Would you like to try to fix those in this PR?

@shahpratham
Copy link
Author

Okay, I can add in this PR.

Removing const from void calculateVolumes(const HPOLYTOPE &HP) and changing to void calculateVolumes(HPOLYTOPE &HP) worked, same for VPOLYTOPE.

For the second one it seems that you are missing mkl

I tried installing it via. sudo apt-get install intel-mkl-full. I'm even getting an error for spdlog in EnvelopeProblemSOS. What should be done?

1

For optimization_spectrahedra, i tried removing const from void compute_reflection(PointType &v, PointType const& r, update_parameters& ) const, but didn't worked.
Getting this error:

2

Except these 3, others are building successfully.

@shahpratham
Copy link
Author

shahpratham commented Mar 7, 2022

Hi @vissarion could you please tell what to do for mkl.h, I tried installing it via sudo apt-get install intel-mkl-full and after reading this, i even tried on non-anaconda system. But it didn't worked

And for spdlog i tried installing sudo apt install libspdlog-dev and even cloned this repo and added proper paths still showed spdlog.cnf/env.h: No such file or directory

@vissarion
Copy link
Member

  • Regarding mkl tests of volesti are using it successfully, could you please follow how it is done in tests?

  • For spdlog you have to clone and build the repo from github (please do not forget to make install to override the version from apt-get); that worked for me locally.

  • For const references, I think it should not be needed to pass a polytope as a non const reference in volume algorithms. @TolisChal @AlexManochis do you remember why this is changed in Volume approximation for spectrahedra #173 ?

@shahpratham
Copy link
Author

Regarding mkl tests of volesti are using it successfully, could you please follow how it is done in tests?

I tried doing like how its done in test/CMakeLists.txt but its still showing:

/usr/include/eigen3/Eigen/src/Core/util/MKL_support.h:58:13: fatal error: mkl.h: No such file or directory
   58 | #   include <mkl.h>

For spdlog you have to clone and build the repo from github (please do not forget to make install to override the version from apt-get); that worked for me locally.

Yes, after this spdlog was installed, but facing same error while doing make ( error: declaration of ‘typedef Vector<T> LHSCB<IPMDouble>::Vector’ changes meaning of ‘Vector’ [-fpermissive]) as in discussion #215.

@shahpratham
Copy link
Author

Hey, @vissarion I'm still facing this issue. Can you please lead me on this? The only thing left is that make is not building correctly on these three files.

@vissarion
Copy link
Member

It seems that this PR is stale for a long time. I marked it stale as a reminder and I will close it is it remains stale after 4 weeks.

@vissarion
Copy link
Member

Closing as inactive.

@vissarion vissarion closed this Jul 11, 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.

Fix examples and CI integration
2 participants