Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
Helmholtz EOS #264
Helmholtz EOS #264
Changes from 68 commits
6ae5bba
224497d
475e383
8a8c642
c7592e1
5d30090
ea0a8cb
92dd13c
ffc0e33
e7a61a4
ebc28f2
8370c8e
bb30cbc
fab0fe8
58cae76
7c7ab75
0fed42e
3c69557
3d30aa4
7ed5bd3
54223f7
8d07a53
40a94a2
4159ea5
59ad99c
7c01149
7824da0
53fee9d
e7c14eb
5fab84d
3e8a6d2
d42d912
4087192
5333e6c
078eabe
2b4d2ef
9881ad9
ccae8df
062085a
bbd77fd
d8355f2
2b06071
65aaa59
f57a6fd
5de620e
ce11267
37fcff3
a72845f
53fdae2
c65b00a
36dd698
56e8835
b82c2a1
7444c86
76c64a0
f812bba
905b207
66deb2a
3f9c1dc
e10d5dd
9e8319a
32fe9d0
9fbfbb6
f5b68e4
b69386f
0806486
0574349
d36eb7a
a4b9a66
ef8c818
fa87671
9cc18af
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
you should add something like
which will copy the data directory to the build directory.
Also will need to add similar in
cmake/install.cmake
for installingThere 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.
Ah, I see this was done in the test directory. Nevermind.
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.
Still need to add a step to install the data. Something like
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.
I have moved the table to a newly created
data
directory. The idea is that e.g. future tabulated EOS can collect the table files there (if they are not publicly available elsewhere). I'm not sure if this is desired, but since the table needs to be included for testing (and it is not publicly available in the required format) and we want to install the table anyway, I thought it might be better to have it plainly available instead of being 'hidden' in thetest
directory. I do, however, have no strong feelings towards either approach and the changes could be easily reverted if a centraldata
directory is unwanted.Also I'm not sure if the configuration of the data directory install location should be documented or if the default
share
location is obvious enough.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.
Can't hurt to document the location of the file.
Large diffs are not rendered by default.
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.
Longer term, I wonder if we should combine all the astro tests into a
singularity_astro
build option or something. That can be for later though. Not this PR.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.
Very pleased with this new version of our integer power function. It's generically recursive, with the even/odd power performance trick, so we don't need the strategic template specializations anymore. And it handles negative integer powers.