Skip to content
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

Merged

Conversation

ax3l
Copy link
Member

@ax3l ax3l commented Jan 9, 2017

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 for radiationFlag.

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 bools in its manual.

To Do

  • RT test
  • we should add a radiation test where RAD_MARK_PARTICLE is larger 1 or RAD_ACTIVATE_GAMMA_FILTER is non-zero to cover that case in the compile suite

@ax3l ax3l added affects latest release a bug that affects the latest stable release bug a bug in the project's code component: plugin in PIConGPU plugin labels Jan 9, 2017
@ax3l ax3l added this to the Next Stable: 0.3.0 / 1.0.0 milestone Jan 9, 2017
@ax3l ax3l requested a review from PrometheusPi January 9, 2017 16:05
Copy link
Member

@PrometheusPi PrometheusPi left a 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) {}
Copy link
Member

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() { }

Copy link
Member Author

@ax3l ax3l Jan 11, 2017

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.

@ax3l
Copy link
Member Author

ax3l commented Jan 11, 2017

@PrometheusPi in case that is interesting for you, you can check with HDFCompass, pip install adios or bpls/bpdump if the data representation of the flag makes some kind of sense to you?

@ax3l ax3l force-pushed the fix-adiosBoolRadFlag branch from 27100a9 to f4a3b85 Compare January 18, 2017 17:34
@PrometheusPi
Copy link
Member

@ax3l What is the status of the ToDo list?

@ax3l
Copy link
Member Author

ax3l commented Jan 24, 2017

I did hope a little that we talk how to test it xD

ax3l added 3 commits January 25, 2017 15:37
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).
@ax3l ax3l force-pushed the fix-adiosBoolRadFlag branch from f4a3b85 to aab2ad4 Compare January 25, 2017 16:09
@ax3l
Copy link
Member Author

ax3l commented Jan 25, 2017

Tested in hypnos:/bigdata/hplsim/development/huebl/bunch-adios-radflag-002/simOutput/bp

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 radiationFlag as expected.

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.

@PrometheusPi PrometheusPi merged commit ab1a761 into ComputationalRadiationPhysics:dev Jan 27, 2017
@ax3l ax3l deleted the fix-adiosBoolRadFlag branch January 27, 2017 17:12
@ax3l ax3l mentioned this pull request Feb 9, 2017
2 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
affects latest release a bug that affects the latest stable release bug a bug in the project's code component: plugin in PIConGPU plugin
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants