Create devcontainer.json #34

niemasd · 2023-11-11T01:10:16Z

With this devcontainer file, VS Code (and GitHub Codespaces) will be able to create the development environment automatically.

stevenweaver · 2023-11-13T19:37:06Z

Dockerfile

+RUN yum -y update && \
+    yum install -y cmake gcc-c++ git make && \
+    sed -i 's/shared(currently_defined_clusters, try_cluster, sequence_lengths, current_sequence, current_clusters, firstSequenceLength, min_overlap)/shared(currently_defined_clusters, try_cluster, sequence_lengths, current_sequence, current_clusters)/g' src/read_reducer.cpp && \
+    sed -i 's/shared(my_distance_estimate,nodeParents,workingNodes,distanceEstimates, step_penalty, min_overlap, resolutionOption, firstSequenceLength, theSequence, left_to_do)/shared(my_distance_estimate,nodeParents,workingNodes,distanceEstimates)/g' src/ShortestPathTN93.cpp


Would you like to just commit the change to the codebase instead of sed? A patch file is another alternative.

niemasd · 2023-11-13T19:43:38Z

Okay, the OpenMP edits have been included in the following commit:

7a55238

I just tested that, starting from the development Docker image, I'm able to successfully compile and run tn93 as follows:

cmake .
make
./tn93 -h

However, I think this edit might break compilation in the Ubuntu environment. @stevenweaver Can you check?

EDIT: Actually, it seems like the "CMake on multiple platforms" check worked, so I think this is ready to merge!

EDIT 2: Never mind; it seems to break in the Ubuntu one. Is there a backwards-compatible syntax for this OpenMP syntax?

stevenweaver · 2023-11-14T15:39:02Z

@niemasd -- allow me some time to get to root of this issue. There seems to be a discrepancy between distributions that extends beyond which compiler version we are using.

niemasd · 2023-11-14T15:40:51Z

@stevenweaver Sounds great; thanks for looking into it!

stevenweaver · 2023-11-17T21:02:20Z

Hi Niema,

This is due to an incompatibility with OpenMP 4.5. Because OpenMP 4.5 standard is old, I don't think it's worthwhile to support it.

We can do the following to use gcc-10:

yum install oracle-epel-release-el8
yum install gcc-toolset-10
scl enable gcc-toolset-10 bash

Then proceed with CMake as normal.

Best,
Steven

niemasd · 2023-11-17T22:01:22Z

Thanks for looking into that! Should we use gcc-10, or should we use the sed one-liner to edit that in the Docker environment?

If sed one-liner, we should be good to go with the other repos
If switch to gcc-10, I'll need to make the change in the other repos' Dockerfiles as well (which should be quick, but just want to make sure that's what we want to do)

stevenweaver · 2023-11-17T22:02:53Z

Well, what I'm worried about with the sed one-liner is that it would remove parallelism from some functions (if I'm reading this correctly), but I haven't checked what the impact on performance would be.

niemasd · 2023-11-17T22:04:08Z

Ah, I didn't realize that! I'll switch over to gcc-10, then. Better safe than sorry! Will create updated PRs ASAP

spond · 2023-11-17T22:07:58Z

Dear @niemasd and @stevenweaver,

I have some code in HyPhy to deal with different OPENMP versions. Take a look at https://github.com/veg/hyphy/blob/c1f1ca1ce0123bc7e20fac39810a7aa8ba26377b/src/core/matrix.cpp#L3987

Best,
Sergei

niemasd · 2023-11-17T22:22:11Z

I reverted the OpenMP edits I had made to the tn93 source code, so it should now compile on the environments it used to, but it now won't compile in the Docker environment. Which of the following should I pursue?

Incorporate @spond's HyPhy fix to get tn93 compiling in the Docker environment, or
Incorporate @stevenweaver's GCC version update in the Docker environment

I would personally lean towards 2, but I am happy with what you two think is best

niemasd · 2023-11-20T18:05:35Z

I'll go ahead and move forward with option 2 (use @stevenweaver's GCC version update), and will create/update PRs accordingly.

niemasd · 2023-11-20T18:09:20Z

@stevenweaver The proposed fixes to use GCC 10 didn't seem to work. Specifically, I updated the Dockerfile as follows:

RUN yum -y update && \
    yum install -y cmake gcc-c++ gcc-toolset-10 git make oracle-epel-release-el8 && \
    scl enable gcc-toolset-10 bash

But I still get the following compilation error:

/workspaces/tn93/src/read_reducer.cpp: In function ‘void handle_a_sequence(StringBuffer&, StringBuffer&, Vector&, Vector&, long int, long int, Vector*)’:
/workspaces/tn93/src/read_reducer.cpp:128:187: error: ‘firstSequenceLength’ is predetermined ‘shared’ for ‘shared’
             #pragma omp parallel for default(none) shared(currently_defined_clusters, try_cluster, sequence_lengths, current_sequence, current_clusters, firstSequenceLength, min_overlap)
                                                                                                                                                                                           ^
/workspaces/tn93/src/read_reducer.cpp:128:187: error: ‘min_overlap’ is predetermined ‘shared’ for ‘shared’
make[2]: *** [CMakeFiles/readreduce.dir/build.make:76: CMakeFiles/readreduce.dir/src/read_reducer.cpp.o] Error 1
make[1]: *** [CMakeFiles/Makefile2:129: CMakeFiles/readreduce.dir/all] Error 2
make: *** [Makefile:136: all] Error 2

I'll investigate; any thoughts?

stevenweaver · 2023-11-20T18:12:15Z

@niemasd -- Do you still have the output of CMake? Is it picking up GCC-10? I used the Dockerfile provided with Visual Studio to confirm that it worked, but it was in an interactive terminal. I'll try running the updated Dockerfile as well.

niemasd · 2023-11-20T18:14:36Z

Here's the complete output; I don't seem to see the GCC version:

[root@codespaces-f9765f tn93]# cmake .
-- Configuring done
-- Generating done
-- Build files have been written to: /workspaces/tn93
[root@codespaces-f9765f tn93]# make
[  2%] Building CXX object CMakeFiles/nucfreqsfasta.dir/src/nuc_freqs_from_fasta.cpp.o
[  4%] Building CXX object CMakeFiles/nucfreqsfasta.dir/src/stringBuffer.cc.o
[  6%] Building CXX object CMakeFiles/nucfreqsfasta.dir/src/tn93_shared.cc.o
[  8%] Linking CXX executable nucfreqsfasta
[  8%] Built target nucfreqsfasta
[ 10%] Building CXX object CMakeFiles/readreduce.dir/src/read_reducer.cpp.o
/workspaces/tn93/src/read_reducer.cpp: In function ‘void handle_a_sequence(StringBuffer&, StringBuffer&, Vector&, Vector&, long int, long int, Vector*)’:
/workspaces/tn93/src/read_reducer.cpp:128:187: error: ‘firstSequenceLength’ is predetermined ‘shared’ for ‘shared’
             #pragma omp parallel for default(none) shared(currently_defined_clusters, try_cluster, sequence_lengths, current_sequence, current_clusters, firstSequenceLength, min_overlap)
                                                                                                                                                                                           ^
/workspaces/tn93/src/read_reducer.cpp:128:187: error: ‘min_overlap’ is predetermined ‘shared’ for ‘shared’
make[2]: *** [CMakeFiles/readreduce.dir/build.make:76: CMakeFiles/readreduce.dir/src/read_reducer.cpp.o] Error 1
make[1]: *** [CMakeFiles/Makefile2:129: CMakeFiles/readreduce.dir/all] Error 2
make: *** [Makefile:136: all] Error 2

At the command line, it seems like GCC-10 is indeed the version in the environment (but not sure if CMake just isn't using it)?

[root@codespaces-f9765f tn93]# g++ --version
g++ (GCC) 10.3.1 20210422 (Red Hat 10.3.1-1.2.0.1)
Copyright (C) 2020 Free Software Foundation, Inc.
This is free software; see the source for copying conditions.  There is NO
warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.

[root@codespaces-f9765f tn93]# gcc --version
gcc (GCC) 10.3.1 20210422 (Red Hat 10.3.1-1.2.0.1)
Copyright (C) 2020 Free Software Foundation, Inc.
This is free software; see the source for copying conditions.  There is NO
warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.

stevenweaver · 2023-11-20T18:15:58Z

Yeah, there must be cached CMake files somewhere, which is really weird if a fresh container is being generated from a Dockerfile.

niemasd · 2023-11-20T18:22:11Z

Ah yup, I was able to show the whole compilation commands as follows:

[root@codespaces-f9765f tn93]# cmake --build . -v
...
/usr/bin/c++ -DVERSION_NUMBER=\"v1.0.12\" -I/workspaces/tn93/src -fopenmp -O3 -std=c++14 -funroll-loops -fopenmp -MD -MT CMakeFiles/readreduce.dir/src/read_reducer.cpp.o -MF CMakeFiles/readreduce.dir/src/read_reducer.cpp.o.d -o CMakeFiles/readreduce.dir/src/read_reducer.cpp.o -c /workspaces/tn93/src/read_reducer.cpp
/workspaces/tn93/src/read_reducer.cpp: In function ‘void handle_a_sequence(StringBuffer&, StringBuffer&, Vector&, Vector&, long int, long int, Vector*)’:
/workspaces/tn93/src/read_reducer.cpp:128:187: error: ‘firstSequenceLength’ is predetermined ‘shared’ for ‘shared’
             #pragma omp parallel for default(none) shared(currently_defined_clusters, try_cluster, sequence_lengths, current_sequence, current_clusters, firstSequenceLength, min_overlap)
                                                                                                                                                                                           ^
/workspaces/tn93/src/read_reducer.cpp:128:187: error: ‘min_overlap’ is predetermined ‘shared’ for ‘shared’
gmake[2]: *** [CMakeFiles/readreduce.dir/build.make:76: CMakeFiles/readreduce.dir/src/read_reducer.cpp.o] Error 1

Checking that /usr/bin/c++ command:

[root@codespaces-f9765f tn93]# /usr/bin/c++ -DVERSION_NUMBER=\"v1.0.12\" -v
Using built-in specs.
COLLECT_GCC=/usr/bin/c++
COLLECT_LTO_WRAPPER=/usr/libexec/gcc/x86_64-redhat-linux/8/lto-wrapper
OFFLOAD_TARGET_NAMES=nvptx-none
OFFLOAD_TARGET_DEFAULT=1
Target: x86_64-redhat-linux
Configured with: ../configure --enable-bootstrap --enable-languages=c,c++,fortran,lto --prefix=/usr --mandir=/usr/share/man --infodir=/usr/share/info --with-bugurl=http://bugzilla.redhat.com/bugzilla --enable-shared --enable-threads=posix --enable-checking=release --enable-multilib --with-system-zlib --enable-__cxa_atexit --disable-libunwind-exceptions --enable-gnu-unique-object --enable-linker-build-id --with-gcc-major-version-only --with-linker-hash-style=gnu --enable-plugin --enable-initfini-array --with-isl --disable-libmpx --enable-offload-targets=nvptx-none --without-cuda-driver --enable-gnu-indirect-function --enable-cet --with-tune=generic --with-arch_32=x86-64 --build=x86_64-redhat-linux
Thread model: posix
gcc version 8.5.0 20210514 (Red Hat 8.5.0-18.0.6) (GCC)

I'll look into seeing how to make sure CMake uses the c++ in PATH (which is the correct one), rather than using /usr/bin/c++:

[root@codespaces-f9765f tn93]# which g++
/opt/rh/gcc-toolset-10/root/usr/bin/g++
[root@codespaces-f9765f tn93]# which c++
/opt/rh/gcc-toolset-10/root/usr/bin/c++

niemasd · 2023-11-20T18:36:31Z

I think I figured it out: the scl enable gcc-toolset-10 bash command seems to only change the selected GCC in the currently running bash environment, not on all bash environments. Instead, I'll add it to the .bashrc in the tn93 Dockerfile so that it gets executed in every bash shell (and will source .bashrc in the Dockerfile so that downstream commands in the Dockerfile have it as well). Will follow up soon.

EDIT: I also changed it to the following command as per some comments from Googling:

source /opt/rh/gcc-toolset-10/enable

niemasd · 2023-11-20T18:42:53Z

@stevenweaver Confirmed that it worked! I've created a new PR with the fix: #36

niemasd added 8 commits November 10, 2023 16:56

Create devcontainer.json

d7bd259

Update Dockerfile

624491b

Update Dockerfile

77830c3

Update Dockerfile

b827f76

Update Dockerfile

9493454

Fix OpenMP issue with GCC 8.5

a00a7c8

Update Dockerfile

f90f3c8

Update Dockerfile

b068fbf

stevenweaver self-requested a review November 13, 2023 19:36

stevenweaver reviewed Nov 13, 2023

View reviewed changes

niemasd added 2 commits November 13, 2023 11:38

Update Dockerfile

411b54d

Update OpenMP usage

7a55238

niemasd added 2 commits November 13, 2023 11:46

Update Dockerfile

0bd6d7b

Update Dockerfile

45c86d2

Revert OpenMP edits (will try to incorporate Sergei's fix)

3d8be03

stevenweaver merged commit b5d0c55 into veg:master Nov 17, 2023
2 checks passed

niemasd mentioned this pull request Nov 20, 2023

Create devcontainer.json veg/hivtrace#95

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Create devcontainer.json #34

Create devcontainer.json #34

niemasd commented Nov 11, 2023 •

edited

Loading

stevenweaver Nov 13, 2023

niemasd Nov 13, 2023

niemasd commented Nov 13, 2023 •

edited

Loading

stevenweaver commented Nov 14, 2023 •

edited

Loading

niemasd commented Nov 14, 2023

stevenweaver commented Nov 17, 2023

niemasd commented Nov 17, 2023

stevenweaver commented Nov 17, 2023

niemasd commented Nov 17, 2023

spond commented Nov 17, 2023

niemasd commented Nov 17, 2023 •

edited

Loading

niemasd commented Nov 20, 2023

niemasd commented Nov 20, 2023

stevenweaver commented Nov 20, 2023

niemasd commented Nov 20, 2023

stevenweaver commented Nov 20, 2023 •

edited

Loading

niemasd commented Nov 20, 2023

niemasd commented Nov 20, 2023 •

edited

Loading

niemasd commented Nov 20, 2023 •

edited

Loading

Create devcontainer.json #34

Create devcontainer.json #34

Conversation

niemasd commented Nov 11, 2023 • edited Loading

stevenweaver Nov 13, 2023

Choose a reason for hiding this comment

niemasd Nov 13, 2023

Choose a reason for hiding this comment

niemasd commented Nov 13, 2023 • edited Loading

stevenweaver commented Nov 14, 2023 • edited Loading

niemasd commented Nov 14, 2023

stevenweaver commented Nov 17, 2023

niemasd commented Nov 17, 2023

stevenweaver commented Nov 17, 2023

niemasd commented Nov 17, 2023

spond commented Nov 17, 2023

niemasd commented Nov 17, 2023 • edited Loading

niemasd commented Nov 20, 2023

niemasd commented Nov 20, 2023

stevenweaver commented Nov 20, 2023

niemasd commented Nov 20, 2023

stevenweaver commented Nov 20, 2023 • edited Loading

niemasd commented Nov 20, 2023

niemasd commented Nov 20, 2023 • edited Loading

niemasd commented Nov 20, 2023 • edited Loading

niemasd commented Nov 11, 2023 •

edited

Loading

niemasd commented Nov 13, 2023 •

edited

Loading

stevenweaver commented Nov 14, 2023 •

edited

Loading

niemasd commented Nov 17, 2023 •

edited

Loading

stevenweaver commented Nov 20, 2023 •

edited

Loading

niemasd commented Nov 20, 2023 •

edited

Loading

niemasd commented Nov 20, 2023 •

edited

Loading