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

[doc]: add prerequisites in README.md #304

Merged
merged 2 commits into from
Mar 31, 2024

Conversation

broccoliSpicy
Copy link
Contributor

This PR tries to fix #303 by adding prerequisites for building Lantern from source code in README.md

@broccoliSpicy broccoliSpicy changed the title add prerequisites in README.md [doc]: add prerequisites in README.md Mar 30, 2024
@broccoliSpicy
Copy link
Contributor Author

after reading
Once a variable is in the cache, its “cache” value cannot normally be modified from a CMakeLists file. The reasoning behind this is that once CMake has put the variable into the cache with its initial value, the user may then modify that value from the GUI. If the next invocation of CMake overwrote their change back to the [set](https://cmake.org/cmake/help/latest/command/set.html#command:set) value, the user would never be able to make a change that CMake wouldn’t overwrite. A set(FOO ON CACHE BOOL "doc") command will typically only do something when the cache doesn’t have the variable in it. Once the variable is in the cache, that command will have no effect.
from cmake cache doc, I think it might be better to specify MARCH_NATIVE=ON explicitly.
like this:

git clone --recursive https://github.com/lanterndata/lantern.git
cd lantern
mkdir build
cd build
cmake -DMARCH_NATIVE=ON ..
make install

I am just worried that the user might build portable binaries first then follow the instructions to build a CPU-specific vectorization one, see that the only difference is cmake .. and only reruns this command, without knowing that MARCH_NATIVE=ON is actually not taking effect, since the settings we have in CMakeLists.txt won't overwrite CMakeCache.txt

@Ngalstyan4 Ngalstyan4 merged commit 33e6bcf into lanterndata:main Mar 31, 2024
24 of 44 checks passed
@Ngalstyan4
Copy link
Contributor

thanks for your contribution, @broccoliSpicy !

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.

[doc]: README.md does not specify prerequisites for build Lantern from source code
2 participants