-
Notifications
You must be signed in to change notification settings - Fork 8
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
dholladay00/separate get sg eos #245
Changes from 30 commits
289f39c
e56b44b
4c948da
8ba8a2a
ef62d59
27a66ee
fffa84c
98ad54c
4d35a16
da798cc
c5df26b
acd3349
1fbcfac
54dfeec
de35b08
7f1997a
30de7d3
53ef4db
cf23fd5
e196b55
be1268d
2080ef6
ecff459
52dc1c9
9151751
3f2cbf9
ea59f83
1c284b2
946720c
fc07762
9b3bdb0
7650792
05231e9
cc6e5d1
4fd4cfd
63456b3
f7736f8
0aab253
1f77a51
1d4fc85
3748a69
0dd0268
ed4e55a
9774621
c9823d2
bdf02e3
a77b090
0e37800
4179042
7129f96
ff334d2
4b6c859
a225052
5ad4058
55ceed2
305f7e6
ed5a121
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 |
---|---|---|
|
@@ -80,7 +80,10 @@ char *StrCat(char *destination, const char *source) { | |
using EosBase<EOSDERIVED>::FillEos; \ | ||
using EosBase<EOSDERIVED>::EntropyFromDensityTemperature; \ | ||
using EosBase<EOSDERIVED>::EntropyFromDensityInternalEnergy; \ | ||
using EosBase<EOSDERIVED>::EntropyIsNotEnabled; | ||
using EosBase<EOSDERIVED>::EntropyIsNotEnabled; \ | ||
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 assume these would be required @Yurlungur, is any of this machinery tested on unmodified Eos's? 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. Just an FYI - I think you can remove this line completely since it's automatically slurped in via inheritance 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 think @jhp-lanl is correct. I think the machinery is checked on unmodified EOS's explicitly. But even if it is not, the "pull out an unmodified EOS" gets automatically tested on unmodified EOS's because that's the base case of the recursion algorithm. |
||
using EosBase<EOSDERIVED>::IsModified; \ | ||
using EosBase<EOSDERIVED>::UnmodifyOnce; \ | ||
using EosBase<EOSDERIVED>::GetUnmodifiedObject; | ||
|
||
/* | ||
This is a CRTP that allows for static inheritance so that default behavior for | ||
|
@@ -426,12 +429,13 @@ class EosBase { | |
} | ||
|
||
// Tooling for modifiers | ||
PORTABLE_FORCEINLINE_FUNCTION | ||
bool IsModified() const { return false; } | ||
PORTABLE_FORCEINLINE_FUNCTION | ||
auto UnmodifyOnce() { return *static_cast<CRTP *>(this); } | ||
PORTABLE_FORCEINLINE_FUNCTION | ||
auto GetUnmodifiedObject() { return *static_cast<CRTP *>(this); } | ||
inline constexpr bool IsModified() const { return false; } | ||
dholladay00 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
inline constexpr decltype(auto) UnmodifyOnce() { return *static_cast<CRTP *>(this); } | ||
|
||
inline constexpr decltype(auto) GetUnmodifiedObject() { | ||
return *static_cast<CRTP *>(this); | ||
} | ||
}; | ||
} // namespace eos_base | ||
} // namespace singularity | ||
|
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 need to change this back, but other than that, can you live with these changes @ktsai7 @Yurlungur ?
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.
@dholladay00 I would probably say to use a global
SINGULARITY_EOS_BUILD_TYPE
variable and use anchors to change the value in the appropriate jobs instead of doing theexport
in the lines of code... it's just easier to see this wayI could add a commit for this if you want or don't fully understand what I'm saying
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.
That would be appreciated @ktsai7 I don't know what an anchor is.