-
Notifications
You must be signed in to change notification settings - Fork 217
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
Fix ADIOS Trait for bool
#1756
Fix ADIOS Trait for bool
#1756
Conversation
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.
Looks good to me, just a question on what might be an optimization.
ADIOS_DATATYPES type; | ||
|
||
PICToAdios() : | ||
type(adios_unsigned_byte) {} |
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 assume ADIOS_DATATYPES
is a type for data-types - is this correct?
If so could you not write something like
constexpr ADIOS_DATATYPES type = adios_unsigned_byte;
PICToAdios() { }
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.
Thanks for the question. It's an enum, or in-code an int.
There is no need to modify that, since this is in a non-sensitive part of the code. The main sensitive part on the host side is large scale data movement / data reorganization and processing, besides that we needs such tunings on the device. For all other places, like here it is just a minor meta data preparation, usability and maintainability first since it does not matter for performance (which in the ADIOS case is the actual write that is scheduled after all the preparations).
It would also unnecessarily refactor the already existing interface we have right now. It is sufficient.
@PrometheusPi in case that is interesting for you, you can check with HDFCompass, |
27100a9
to
f4a3b85
Compare
@ax3l What is the status of the ToDo list? |
I did hope a little that we talk how to test it xD |
For default I/O and checkpointing via ADIOS, simulations using our in situ radiation plugin with selected particles (by stride or gamma) did not compile since ADIOS' C API does not define a `bool` type for `radiationFlag`. This adds a trait assuming it can be casted to one byte.
Fix that the usage of `math::` is ambigous as soon as the radiation gamma filter is used.
Test the gamma filter (and `radiationFlag` usage) in the Bunch example since it was not yet covered by the compile suite (and broken).
f4a3b85
to
aab2ad4
Compare
Tested in At least from bpls simData_0.bp -d "/data/0/particles/e/radiationFlag" | less
bpls simData_2500.bp -d "/data/2500/particles/e/radiationFlag" | less there seem to be zeroes (at t=0) and ones (for all later stes) in it for Same picture with python import adios as ad
f = ad.File("simData_2500.bp")
print( f["/data/2500/particles/e/radiationFlag"][42] )
#1
f = ad.File("simData_0.bp")
print( f["/data/0/particles/e/radiationFlag"][42] )
#0 so imo we are good to go. |
For default I/O and checkpointing via ADIOS, simulations using our in situ radiation plugin with selected/subsets of particle species (by stride or gamma) did not compile since ADIOS' C API does not define a
bool
type forradiationFlag
.This adds a trait assuming it can be casted to one byte. It's nasty but I have no idea what to use instead since ADIOS has nothing on
bool
s in its manual.To Do
RAD_MARK_PARTICLE
is larger 1 orRAD_ACTIVATE_GAMMA_FILTER
is non-zero to cover that case in the compile suite