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

More changes for conda packaging #112

Merged
merged 4 commits into from
Dec 9, 2019
Merged

Conversation

robertodr
Copy link
Contributor

Changes motivated by packaging the library for Anaconda

Changes motivated by packaging the library for Anaconda
@robertodr
Copy link
Contributor Author

I can actually get a working MRCPP conda package:

conda create -n foo mrcpp=*=mpi* cmake -c nostress -c conda-forge --yes

I am able to compile the poisson example. Running works almost fine, I get:

Screenshot from 2019-12-03 14-36-11

and have been unable to solve it...

@stigrj
Copy link
Contributor

stigrj commented Dec 3, 2019

Hmm, we might have a bug. Have you tried other examples?

@MinazoBot
Copy link

2 Warnings
⚠️ There are code changes, but no corresponding tests. Please include tests if this PR introduces any modifications in behavior.
⚠️ Consider adding supporting documentation to this change. Documentation sources can be found in the doc directory.

Generated by 🚫 Danger

@codecov
Copy link

codecov bot commented Dec 3, 2019

Codecov Report

Merging #112 into master will increase coverage by 0.01%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #112      +/-   ##
==========================================
+ Coverage   72.22%   72.24%   +0.01%     
==========================================
  Files         139      139              
  Lines        5955     5959       +4     
==========================================
+ Hits         4301     4305       +4     
  Misses       1654     1654
Impacted Files Coverage Δ
src/core/MWFilter.h 100% <ø> (ø) ⬆️
src/core/CrossCorrelation.h 100% <ø> (ø) ⬆️
src/treebuilders/PHCalculator.cpp 100% <100%> (ø) ⬆️
src/core/CrossCorrelation.cpp 71.42% <100%> (+0.59%) ⬆️
src/core/MWFilter.cpp 50.84% <100%> (+0.42%) ⬆️
src/treebuilders/BSCalculator.cpp 98.5% <100%> (+0.02%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update dc2ec19...db4e10e. Read the comment docs.

@robertodr
Copy link
Contributor Author

Nope, I was lazy. I'll try all the examples and see. Note that we have two variants of the package: OpenMP and OpenMP+MPI and it happens with both versions. I haven't understood how to make a serial version and haven't really investigated (as I thought it's rather pointless for distribution)

@robertodr
Copy link
Contributor Author

conda-build compiles with -fstack-protector-strong which I think makes the (probable) bug appear.

@robertodr
Copy link
Contributor Author

robertodr commented Dec 3, 2019

Update

The package still needs a little bit more work. Hold on merging!

  • When compiling mpi_shared_tree.cpp I get:
CMakeFiles/mpi_shared_tree.dir/mpi_shared_tree.cpp.o: In function `main':
mpi_shared_tree.cpp:(.text+0x352): undefined reference to `mrcpp::SharedMemory::SharedMemory(int, int)'
mpi_shared_tree.cpp:(.text+0x40e): undefined reference to `void mrcpp::share_tree<3>(mrcpp::FunctionTree<3>&, int, int, int)'
mpi_shared_tree.cpp:(.text+0x648): undefined reference to `void mrcpp::share_tree<3>(mrcpp::FunctionTree<3>&, int, int, int)'

UPDATE I think this was due to the MRCPP::mrcpp target not exporting its dependency on MPI properly. I am now testing a fix.

  • When compiling mpi_send_tree.cpp I get:
CMakeFiles/mpi_send_tree.dir/mpi_send_tree.cpp.o: In function `main':
mpi_send_tree.cpp:(.text+0x5d8): undefined reference to `void mrcpp::send_tree<3>(mrcpp::FunctionTree<3>&, int, int, int, int)'
mpi_send_tree.cpp:(.text+0x60d): undefined reference to `void mrcpp::recv_tree<3>(mrcpp::FunctionTree<3>&, int, int, int, int)'

UPDATE I think this was due to the MRCPP::mrcpp target not exporting its dependency on MPI properly. I am now testing a fix.

  • When compiling mpi_matrix.cpp I get:
CMakeFiles/mpi_matrix.dir/mpi_matrix.cpp.o: In function `main':
mpi_matrix.cpp:(.text+0x570): undefined reference to `void mrcpp::send_tree<3>(mrcpp::FunctionTree<3>&, int, int, int, int)'
mpi_matrix.cpp:(.text+0x5a5): undefined reference to `void mrcpp::recv_tree<3>(mrcpp::FunctionTree<3>&, int, int, int, int)'

UPDATE I think this was due to the MRCPP::mrcpp target not exporting its dependency on MPI properly. I am now testing a fix.

Of the compiled examples, all but scf smash the stack. scf gives:


============================================================
                        Running SCF
------------------------------------------------------------
 Iter      E_np1          dE_n      ||phi_np1||   ||dPhi_n||
------------------------------------------------------------
  1  -3.5389437389e-01  1.5e-01   1.0341694208e+00  3.7e-01
free(): invalid pointer
Aborted (core dumped)

@stigrj
Copy link
Contributor

stigrj commented Dec 3, 2019

I'm not able to reproduce the stack error on my laptop. Which compiler is used on conda?

@robertodr
Copy link
Contributor Author

robertodr commented Dec 3, 2019 via email

@robertodr
Copy link
Contributor Author

This is the full list of compiler flags used with conda build:

[1/91] $BUILD_PREFIX/bin/x86_64-conda_cos6-linux-gnu-c++  -DHAVE_OPENMP -Dmrcpp_EXPORTS -I../src -Iinclude -isystem $PREFIX/include/eigen3 -fvisibility-inlines-hidden -std=c++17 -fmessage-length=0 -march=nocona -mtune=haswell -ftree-vectorize -fPIC -fstack-protector-strong -fno-plt -O2 -ffunction-sections -pipe -isystem $PREFIX/include -fdebug-prefix-map=$SRC_DIR=/usr/local/src/conda/mrcpp-1.0.2 -fdebug-prefix-map=$PREFIX=/usr/local/src/conda-prefix -O3 -DNDEBUG -fPIC   -fopenmp -std=c++14 -MD -MT src/CMakeFiles/mrcpp.dir/core/CrossCorrelationCache.cpp.o -MF src/CMakeFiles/mrcpp.dir/core/CrossCorrelationCache.cpp.o.d -o src/CMakeFiles/mrcpp.dir/core/CrossCorrelationCache.cpp.o -c ../src/core/CrossCorrelationCache.cpp

@robertodr
Copy link
Contributor Author

I think the stack smashing was a result of not correctly exporting the OpenMP dependency in the MRCPP::mrcpp CMake target.

@stigrj
Copy link
Contributor

stigrj commented Dec 9, 2019

So, what should really be in the VERSION file at this point?

@robertodr
Copy link
Contributor Author

I assume we only want to package tagged releases, so it should be the release number.

@stigrj
Copy link
Contributor

stigrj commented Dec 9, 2019

But this is not a tagged release

@stigrj stigrj merged commit d343073 into MRChemSoft:master Dec 9, 2019
@robertodr robertodr deleted the conda-forge branch December 10, 2019 09:11
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