Add header files to support independent compilation of volesti headers

[Neel-Shah-29]

This PR fixes #184 According to the issue , The cmath header file was not included in boundary_cdhr_walk.hpp , which was giving errors of pow , sqrt functions. So i have made the necessary changes. Kindly suggest if any other changes needs to be done.

[codecov[bot]]

Codecov Report

Merging #212 (37192bd) into develop (2e6572d) will not change coverage. The diff coverage is n/a.

:exclamation: Current head 37192bd differs from pull request most recent head 404c3f2. Consider uploading reports for the commit 404c3f2 to get more accurate results

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

Impacted Files	Coverage Δ
include/convex_bodies/vpolyintersectvpoly.h	70.66% <ø> (ø)
include/ode_solvers/leapfrog.hpp	50.00% <ø> (ø)
include/ode_solvers/randomized_midpoint.hpp	69.00% <ø> (ø)
include/random_walks/boundary_cdhr_walk.hpp	93.93% <ø> (ø)
include/random_walks/boundary_rdhr_walk.hpp	68.00% <ø> (ø)
...ks/gaussian_hamiltonian_monte_carlo_exact_walk.hpp	54.83% <ø> (ø)
include/random_walks/gaussian_helpers.hpp	68.75% <ø> (ø)
include/random_walks/gaussian_rdhr_walk.hpp	54.83% <ø> (ø)
...lude/random_walks/hamiltonian_monte_carlo_walk.hpp	69.44% <ø> (ø)
include/random_walks/langevin_walk.hpp	43.33% <ø> (ø)
... and 3 more

[Neel-Shah-29]

Thanks for this PR.

Did you check that now the header files compile independently? Adding toy examples that include headers files of the library and compile independently would be helpful.

Yes some examples are running correctly but i got an error while building Try1.cpp which is mentioned in the issue #184

This error will be resolved my including cmath in sphere.hpp and including bits/stdc++.h in boundary_cdhr_walk.hpp. This solution worked fine for me!

All the tests and build ran successfully after making that change.Now the examples compile independently without explicitly including cmath in the example program.

So should i make the changes and put a commit for the same ?

[Neel-Shah-29]

Thanks! Some thoughts here.

• I would suggest not including bits/stdc++.h since it is not standard and will include a lot of unnecessary headers.
• Also your solution currently partially solves the issue; see what happens if one tries to include boundary_rdhr_walk.hpp instead of random_walks.hpp. Similar, issues are expected with some other walks too e.g. boltzmann_hmc_walk.hpp.

• I have removed bits/stdc++.h and instead added utility header file which was required for using pair in boundary_cdhr_walk.hpp and boundary_rdhr_walk.hpp
• I was getting errors of list in boltzmann_hmc_walk.hpp on including it in Try1.cpp so i included list in that file.
• I added cmath in sphere.hpp as it was required according to above comment

Do i need to check importing all walks files in example and try compiling them independent of header files or this much changes are enough?

Kindly suggest me if any changes are to be done. Thanks!

[vissarion]

Kindly suggest me if any changes are to be done. Thanks!

Could you please check all files in random_walks one by one if they compile alone and add the necessary header files to them?

[Neel-Shah-29]

Kindly suggest me if any changes are to be done. Thanks!

Could you please check all files in random_walks one by one if they compile alone and add the necessary header files to them?

Yes sure i will check for all of them.

I have tried compiling exponential_hamiltonian_monte_carlo_exact_walk.hpp than i am getting the following error. Similar error is coming while compiling gaussian_hamiltonian_monte_carlo_exact_walk.hpp also.

It says compute_diameter is not included in scope ,Can you please tell the header file that needs to be included in both of these files to resolve this error.

[Neel-Shah-29]

Kindly suggest me if any changes are to be done. Thanks!

Could you please check all files in random_walks one by one if they compile alone and add the necessary header files to them?

Yes sure i will check for all of them.

I have tried compiling exponential_hamiltonian_monte_carlo_exact_walk.hpp than i am getting the following error. Similar error is coming while compiling gaussian_hamiltonian_monte_carlo_exact_walk.hpp also.

It says compute_diameter is not included in scope ,Can you please tell the header file that needs to be included in both of these files to resolve this error.

This issue is resolved by including uniform_billiard_walk.hpp in both the files as compute_diameter is declared in over there.

[vissarion]

This issue is resolved by including uniform_billiard_walk.hpp in both the files as compute_diameter is declared in over there.

OK, please add a comment on the code on why you included that header file. What about all other header files? Are they OK?

[vaithak]

Hi @vissarion, I think this check should be automated so that this is ensured in every PR. I find this stackoverflow answer helpful: https://stackoverflow.com/a/57900525/7317290. @Neel-shah-29 can you check this out?

[Neel-Shah-29]

Hi @vissarion, I think this check should be automated so that this is ensured in every PR. I find this stackoverflow answer helpful: https://stackoverflow.com/a/57900525/7317290. @Neel-Shah-29 can you check this out?

Yes sure i will look into this answer.

This issue is resolved by including uniform_billiard_walk.hpp in both the files as compute_diameter is declared in over there.

OK, please add a comment on the code on why you included that header file. What about all other header files? Are they OK?

Hello @vissarion I have added all the necessary header files in all the files of random_walks and checked compiling all of them independently.

All the files under random_walks compiles independently now according to me. Please review the PR and suggest if any necessary changes needs to be made. Thanks!

[vissarion]

Hi @Neel-Shah-29 thanks. Would you like to try the automated procedure proposed by @vaithak ?

[Neel-Shah-29]

Hi @Neel-Shah-29 thanks. Would you like to try the automated procedure proposed by @vaithak ?

Yes i can try that approach as well, but can this PR be merged first and then i try that approach, as issue #184 can be resolved by this PR. I will submit another PR for that approach,is that ok?

[vaithak]

Hi @Neel-Shah-29, I would prefer that the automated check be done in this PR only, reason being that whenever a bug is solved, the corresponding PR should add some form of check/tests ensuring that this bug won't happen again in the future. This check will ensure that, also this will be make it easy for the reviewers to verify that the problem is actually solved for all the header files.

[Neel-Shah-29]

Hello @vaithak i have pushed the code after adding the custom test kindly check whether its ok or do i need to make any changes. After running make chkdeps i am getting the following output

[vaithak]

Hi @Neel-Shah-29, if you see your log message no target is made for any of the header files. You would have to change the glob expression for searching files. I think this should work:

file(GLOB_RECURSE HDR_ROOT "../include/*.h" "../include/*.hpp")

[Neel-Shah-29]

Hello @vaithak this solution worked great , i am getting target to all header files successfully as seen in below screenshot. but i got an error regarding some target files already exists in /home/neel/volesti/test in my local computer but otherwise everything works fine! I am pushing a commit after doing the changes if any further corrections needs to be done than kindly report it to me

[vaithak]

@Neel-Shah-29, I think that is because of only using filenames without extensions for creating the custom targets, you can also include the extension for naming the target. You will have to refer to cmake's documentation on how that can be done.

[Neel-Shah-29]

@Neel-Shah-29, I think that is because of only using filenames without extensions for creating the custom targets, you can also include the extension for naming the target. You will have to refer to cmake's documentation on how that can be done.

Hello @vissarion @vaithak ,This problem was solved by replacing NAME_WE by NAME as follows:- get_filename_component(HDR_WE ${HDR} NAME) All the files are included with its extension

But i got an error regarding Eigen/Dense: No such file or directory . I tried to find in which file Eigen/Dense was declared but i didn't find it. Can you please help me resolve the following error .

[vaithak]

Hi @Neel-Shah-29. I suspected this would happen as a single header file can include other header files. A fix for this can be to provide the path to essential header files while compiling this custom target. This can be done by adding the following into the COMMAND argument of add_custom_target.

-I../external/_deps/eigen-src -I../external/_deps/boost-src -I../include

This may also require setting WORKING-DIRECTORY of add_custom_target as we are using relative paths with -I

[Neel-Shah-29]

Hi @vaithak The solution given by you worked fine for me , i have added the WORKING_DIRECTORY and argument in COMMAND as mentioned above but i got a error for the header file utils.h. Can you suggest what can be its possible solution.

[vaithak]

The issue is same here, you have to provide the include directories for these header files.

[Neel-Shah-29]

Hello @vaithak , I have made the necessary changes but since the WORKING_DIRECTORY is not set properly the circle ci tests is failing. It runs properly on my local computer but here it shows error, what can be the possible WORKING_DIRECTORY for running circle ci test.

[vaithak]

You can use CMAKE_SOURCE_DIR variable to set the absolute path of working directory.

[Neel-Shah-29]

Hey @vaithak I have made the above changes on my local computer , I will be pushing a commit in a while I am getting the above error that variable_set.h is not included , i am not able to find this header file in any of the sections in the codebase,What should i include to rectify this error?

[vissarion]

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.