-
Notifications
You must be signed in to change notification settings - Fork 9
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
Analytic electron EOS and zsplit modifier #444
Changes from 34 commits
2b258e4
6167b75
4b4a2fa
3d18cae
7fba185
2512249
8085e61
7e9a7ec
54c8a25
aaa57cc
0dfef59
a0a8069
c692221
98fb444
08172b2
95c60cd
3b030b1
7886d11
dc803ed
a0f00c4
c43afcc
d282d09
090559e
d80e40c
780fbd4
af1e70a
ed6622f
39990f8
6f6b6f8
830bb5e
d0dcacd
bc721f8
fc3ee77
0e9f910
2c013c3
7368490
4e93e12
551fc3e
6e255b9
5042643
03d646d
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -9,7 +9,7 @@ on: | |
jobs: | ||
formatting: | ||
name: Check Formatting | ||
runs-on: ubuntu-latest | ||
runs-on: ubuntu-22.04 | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Needed until we update clang-format to clang-17 |
||
|
||
steps: | ||
- name: Checkout code | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Changes in this file required to deal with out-of-memory failure when linking the full fortran interface. |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -21,13 +21,15 @@ jobs: | |
- name: install dependencies | ||
run: | | ||
sudo apt-get update -y -qq | ||
sudo apt-get install -y --allow-downgrades --allow-remove-essential --allow-change-held-packages -qq build-essential gfortran libhdf5-serial-dev | ||
sudo apt-get install -y --allow-downgrades --allow-remove-essential --allow-change-held-packages -qq build-essential gfortran libhdf5-serial-dev binutils-gold | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'll defer to @rbberger @dholladay00 and @mauneyc-LANL on their opinions here There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah i think for now we don't need to change it elsewhere. In this particular CI test, this, GNU ld is too memory inefficient and the test doesn't finish without gold. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I could totally be wrong, but I have a bit of a hunch that among all the changes made, only the switch to There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I was actually impressed by |
||
pip install numpy | ||
pip install h5py | ||
- name: build and run tests | ||
run: | | ||
mkdir -p bin | ||
cd bin | ||
ulimit -m unlimited | ||
ulimit -v unlimited | ||
Comment on lines
+31
to
+32
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Not sure if this does anything inside a docker container, but it doesn't hurt. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. But we could also be minimalistic if desirable... see above. No strong feelings here. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'll leave it. In worst case it does nothing. |
||
mkdir -p ${HOME}/install | ||
cmake -DCMAKE_INSTALL_PREFIX=${HOME}/install \ | ||
-DSINGULARITY_USE_SPINER=ON \ | ||
|
@@ -38,11 +40,14 @@ jobs: | |
-DSINGULARITY_USE_HELMHOLTZ=ON \ | ||
-DSINGULARITY_TEST_HELMHOLTZ=ON \ | ||
-DSINGULARITY_FORCE_SUBMODULE_MODE=ON \ | ||
-DSINGULARITY_USE_V_AND_V_EOS=OFF \ | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Removes |
||
-DSINGULARITY_PLUGINS=$(pwd)/../example/plugin \ | ||
-DCMAKE_LINKER=ld.gold \ | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The gold linker in question |
||
-DCMAKE_BUILD_TYPE=Release \ | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Compared to the default build type, which is There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm surprised that's such a huge memory hit to be honest. Honestly though, I find that if I really need debug symbols, I'm compiling with full There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah. I agree. Is also only for this test. The default build is still "RelWithDebInfo." |
||
.. | ||
#-DSINGULARITY_TEST_PYTHON=ON \ | ||
#-DSINGULARITY_TEST_STELLAR_COLLAPSE=ON \ | ||
#.. | ||
make | ||
make -j4 | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. parallel build. |
||
make install | ||
ctest --output-on-failure |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -85,6 +85,13 @@ cmake_dependent_option( | |
SINGULARITY_USE_HELMHOLTZ "Include Helmholtz equation of state" OFF | ||
"SINGULARITY_USE_SPINER;SINGULARITY_USE_SPINER_WITH_HDF5" OFF) | ||
|
||
# Enables polynomial, carnahan starling, and NobleAbel in variant. | ||
# Off by default, as they are not in the default python bindings or used | ||
# by default in downstream codes. | ||
option(SINGULARITY_USE_V_AND_V_EOS | ||
"Add analytic EOSs for V&V to the variant" | ||
OFF) | ||
|
||
# testing options | ||
option(SINGULARITY_BUILD_TESTS "Compile tests" OFF) | ||
|
||
|
@@ -332,6 +339,9 @@ endif() | |
if(SINGULARITY_USE_HELMHOLTZ) | ||
target_compile_definitions(singularity-eos_Interface INTERFACE SINGULARITY_USE_HELMHOLTZ) | ||
endif() | ||
if (SINGULARITY_USE_V_AND_V_EOS) | ||
target_compile_definitions(singularity-eos_Interface INTERFACE SINGULARITY_USE_V_AND_V_EOS) | ||
endif() | ||
if(SINGULARITY_USE_SPINER_WITH_HDF5) | ||
target_compile_definitions(singularity-eos_Interface INTERFACE SINGULARITY_USE_SPINER_WITH_HDF5) | ||
endif() | ||
|
@@ -440,8 +450,8 @@ if(SINGULARITY_BUILD_TESTS) | |
FetchContent_Declare( | ||
Catch2 | ||
GIT_REPOSITORY https://github.com/catchorg/Catch2.git | ||
# or later is fine too | ||
GIT_TAG v3.0.1) | ||
# jmm: updated Dec 17, 2024 to avoid build errors on modern gcc | ||
GIT_TAG v3.7.1) | ||
Comment on lines
+453
to
+454
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Catch2 v3.0.1 does not compile on ubuntu 24.04. Update to 3.7.1 to rescue it. |
||
FetchContent_MakeAvailable(Catch2) | ||
list(APPEND CMAKE_MODULE_PATH ${Catch2_SOURCE_DIR}/contrib) | ||
endif() | ||
|
@@ -560,6 +570,13 @@ target_compile_options( | |
-use_fast_math | ||
> # release | ||
> # cuda | ||
|
||
# Suppresses annoying ABI notes. See: | ||
# https://stackoverflow.com/questions/52020305/what-exactly-does-gccs-wpsabi-option-do-what-are-the-implications-of-supressi | ||
$<$<AND:$<CXX_COMPILER_ID:GNU>,$<COMPILE_LANGUAGE:CXX>>:-Wno-psabi> | ||
# `-Wclass-memaccess now default with -Wall but we explicitly | ||
# manage this ourselves in our serialization routines. | ||
$<$<AND:$<CXX_COMPILER_ID:GNU>,$<COMPILE_LANGUAGE:CXX>>:-Wno-class-memaccess> | ||
Comment on lines
+577
to
+579
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is apparently a new warning. Since we sometimes build with |
||
) | ||
if (SINGULARITY_STRICT_WARNINGS) | ||
target_compile_options(singularity-eos_Interface INTERFACE | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
builds the python bits faster. Speeds up doc build time by a factor of 4ish