-
Notifications
You must be signed in to change notification settings - Fork 15
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
Close #152 Represent Bools as Enums #153
Close #152 Represent Bools as Enums #153
Conversation
|
@ax3l Yes, I will check this out later today. Would be great if it works. |
I compiled PIConGPU with libSplash from this pull. When writing the checkpoint of time step
|
cfc9765
to
a9b9bce
Compare
fixed, let us add the |
d975023
to
00d4907
Compare
@PrometheusPi tests added and it should work now. can you please give it a try with your PIConGPU example again? |
@ax3l I am already on it. |
@f-schmitt-zih ready for review! :) |
@@ -35,8 +35,11 @@ class ColTypeBool : public CollectionType | |||
|
|||
ColTypeBool() | |||
{ | |||
const hsize_t dim[] = {sizeof (bool)}; | |||
this->type = H5Tarray_create(H5T_NATIVE_B8, 1, dim); | |||
this->type = H5Tenum_create(H5T_NATIVE_INT8); |
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.
same 1byte memory footprint as before, but now an 8-bit native (short) int: link
- add comment above?
af6779b
to
42fe24f
Compare
👍 great job @ax3l - it works !!! |
awesome, thanks! |
if (boolRead[i] != boolWrite[i]) | ||
{ | ||
#if defined TESTS_DEBUG | ||
std::cout << i << ": " << boolRead[i] << " != exptected " << boolWrite[i] << std::endl; |
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.
typo, "expected"
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.
Ha, copy paste typo! Thanks! :)
9b12e46
to
e2a9ec4
Compare
I also added on a side note: if someone wants to test parallel hdf5 reads, there are two issues in |
@f-schmitt-zih ready for final review/merge if you like. Run-Time tests are all clean :) |
e2a9ec4
to
fe01bae
Compare
- read/write a bool field in C++ - read a bool field with h5py - copy test scripts to build dir
Close #152 Represent Bools as Enums
Great job! |
Else it does not identify those as `bool_` but as `int8`. Regression for ComputationalRadiationPhysics#153 (still in `dev`).
Else it does not identify those as `bool_` but as `int8`. Regression for ComputationalRadiationPhysics#153 (still in `dev`). Increases format by a minor since the structure is still the same but the identification in `h5py` is improved (and it is still not in a stable release).
Else it does not identify those as `bool_` but as `int8`. Regression for ComputationalRadiationPhysics#153 (still in `dev`). Increases format by a minor since the structure is still the same but the identification in `h5py` is improved (and it is still not in a stable release).
Proposal to change the internal representation of
bool
values to be compatible withh5py
as documented in #152.To Do
h5py
compatibility tests to.travis.yml
/./run_tests
sizeof (bool)
which is1 Byte
vs. real size of8 Bytes
after conversation - correct in all cases (add a read/write test?)1.2.5
milestone would be ok, too. But it looks a bit odd to make such a change in arevision
- so go for the1.3
target version?