openPMD domain extension for raytracing

[CFGrote]

hi mads, here some comments on your NRAYTRACING extension:

Overall, this is a very good work and I'm really happy how everything is falling into places. The extension is clearly defined and immediately understandable even to non-experts.

In some places your extension deviates from the original standard. I may overlook if this is out of necessity or if you can make it more in line with the original.

• vectors (e.g. position, velocity, etc) : the original standard suggests to store each component as a separate dataset (see e.g. https://github.com/PaNOSC-ViNYL/openPMD-standard/blob/latest/STANDARD.md#required-attributes-for-each-mesh-record). Unless you have a specific reason to deviate from this, I would suggest to stick with it.
• time and time offset: the original standard requests to define the time and time offset at the basePath level, see https://github.com/PaNOSC-ViNYL/openPMD-standard/blob/latest/STANDARD.md#required-attributes-for-the-basepath. Please make sure that your time related datasets are consistent with the basePath level attributes and non-redundant.
• please also add the unitDimension tuples and unitSI scaling factors to your records and recordComponents, see https://github.com/PaNOSC-ViNYL/openPMD-standard/blob/latest/STANDARD.md#required-for-each-record-component and below.

Again, very good work, I'm sure we can smooth out the remaining hickups in due time.

Best regards, Carsten

[mads-bertelsen]

@CFGrote, thanks for the comments!

• No reason not to split vectors into different records for each component. It does however produce some issues with required / optional. For an optional vector like the Spin, what happens if someone only specifies 2 of the 3 dimensions? It seems more appropriate to have the required/optional flag on a vector. Perhaps I just don't see where this information should be stored to affect all components?
• The way the openPMD standard treats time seems more appropriate for meshes than particles. We would for example dump all neutrons that reach a certain plane to disk, and it would take them a different amount of time to reach that, so this time individual for each particle. This does not seem possible to do with the standard time definition of openPMD.
• I have added unitDimension and unitSI to all records, hope they are placed correctly. I see no SI unit available for neutron intensity, but it may be possible to construct?

[CFGrote]

hi mads, very good. I will review once more. I think we're in good shape here and can finish the remaining bits until the deliverable deadline.

carsten

On 10/10/19 4:40 PM, mads-bertelsen wrote:

@CFGrote https://github.com/CFGrote, thanks for the comments!

• No reason not to split vectors into different records for each component. It does however produce some issues with required / optional. For an optional vector like the Spin, what happens if someone only specifies 2 of the 3 dimensions? It seems more appropriate to have the required/optional flag on a vector. Perhaps I just don't see where this information should be stored to affect all components?
• The way the openPMD standard treats time seems more appropriate for meshes than particles. We would for example dump all neutrons that reach a certain plane to disk, and it would take them a different amount of time to reach that, so this time individual for each particle. This does not seem possible to do with the standard time definition of openPMD.
• I have added unitDimension and unitSI to all records, hope they are placed correctly. I see no SI unit available for neutron intensity, but it may be possible to construct?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/PaNOSC-ViNYL/ViNYL-project/issues/15?email_source=notifications&email_token=ADWCQR2AZEUYRHDXLEPJ6Y3QN45HJA5CNFSM4GQLBPI2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEA4TINI#issuecomment-540619829, or unsubscribe https://github.com/notifications/unsubscribe-auth/ADWCQRYYNF7G3YMCHVABIGTQN45HJANCNFSM4GQLBPIQ.

[mads-bertelsen]

The merge request of the neutron raytracing domain extension with our fork of the openPMD framework has now been merged.