Skip to content
Snippets Groups Projects

Add a description and a scale-factor exponent to each individual array written to the snapshots.

Merged Matthieu Schaller requested to merge snapshot_meta_data into master

@lhausammann @jkeger @jborrow

I am working on expanding a bit the meta-data of the fields in the snapshots. One thing which is currently incorrect is the scale-factor exponent. In some cases, it cannot be deduced from the units themselves so it has to be a value provided by the person implementing that scheme. While I am at it, I am also adding a description of the field. This allows us to go beyond just the name.

As you can see, I expand the io_make_output_field() macro to take two more arguments:

  • The scale-factor exponent required to convert that field to physical units.
  • A string for the description.

It is a bit of a pain to adapt all the schemes so I wanted to gather your opinion on this before going ahead.

(Note that SWIFT does not do any conversion from internal units to physical. The scale-factor exponent given there is only to inform the user about what to do if they want to do it themselves.) In the case of planetary-SPH, only the description makes sense I suppose. :)

Also fixes #593 (closed).

Edited by Matthieu Schaller

Merge request reports

Loading
Loading

Activity

Filter activity
  • Approvals
  • Assignees & reviewers
  • Comments (from bots)
  • Comments (from users)
  • Commits & branches
  • Edits
  • Labels
  • Lock status
  • Mentions
  • Merge request status
  • Tracking
  • Nice idea, I am always in favor of improving the snapshots.

    If you need help to do all the schemes, feel free to ask.

  • I'll probably request your help for the GEAR-specific fields, yes. :smile:

    Was there anything else you could think of adding to the meta-data while I am looking at this?

  • Looks nice :) If you're looking for more to add you could always include the units as M L T powers for completeness? And/or even the variable type of float/double/longlong. I guess most any way people could use to see the info would already show that somewhere, but an easy full reference description might be nice.

    Also it'd be prettier if the phrasing is the same for each. At the moment there's a mix of singular and plural and different punctuation e.g. "masses" but "density" not "densities", and no "of the particles" for u. Might as well be consistent before putting in the effort on each scheme.

  • Thanks!

    If you're looking for more to add you could always include the units as M L T powers for completeness?

    That's already there. :smile: (as of last week)

    And/or even the variable type of float/double/longlong

    That's also there as part of hdf5's own meta-data.

    Also it'd be prettier if the phrasing is the same for each. At the moment there's a mix of singular and plural and different punctuation e.g. "masses" but "density" not "densities", and no "of the particles" for u. Might as well be consistent before putting in the effort on each scheme.

    Agreed. I'll propose a list of terms and strings and we can then iterate on that.

    Edited by Matthieu Schaller
  • Matthieu Schaller added 455 commits

    added 455 commits

    Compare with previous version

  • @jborrow @jkeger do you have a guideline regarding singular/plural, etc. ?

    My preference was to have no spaces and use CamelCase.

  • I don't really mind singular or plural, if forced I guess I'd pick singular to avoid the rare plural ambiguities?

    Personally I always prefer underscores and lower case (apart from proper capitals) but no biggie.

  • And sure hdf5 has the type metadata, but if there's going to be a string description anyway I feel it might as well be over-complete so everything's in just the one place to glance at easily once you see it

  • added 2 commits

    • 14e71980 - 1 commit from branch master
    • 20e2cd54 - Add a description and a scale-factor exponent to each individual array written to the snapshots.

    Compare with previous version

  • added 1 commit

    • ba67300e - Print the description of the output fields as comments in the output list yaml file.

    Compare with previous version

  • added 1 commit

    • f1a71a73 - Crash in the i/o code with debugging checks if a field description is left empty.

    Compare with previous version

  • added 1 commit

    • 6b39840a - Also add the new meta-data to the output when using the conversio functions.

    Compare with previous version

  • added 1 commit

    • 5debd18b - Initialize arrays of characters before using them in the meta-data.

    Compare with previous version

  • added 1 commit

    • b578d4f6 - Write the unit conversion string as a comment in the output list YAML.

    Compare with previous version

  • Unless someone shouts loudly, I'll go with singular, CamelCase words as field names.

  • You've still got mixed single and plural, everything else looks good.

    Both the fields with e.g. "Velocities" but "SmoothingLength", but more weird-looking is the description with e.g. "Temperature of the particles" which makes it sound like they all have just one temperature. So either "Temperatures of the particles" or "Temperature of each particle" for each variable

  • Are we not concerned about breaking backwards compatibility with Gadget based tools and scripts? We would also need to change the reader routines and so we would no longer be able to read 'Gadget' HDF5 files.

    If we are to break backwards compatibility, would it then not be best to do it in an obvious way (replacing camel case with underscores)?

    Edited by Josh Borrow
  • You've still got mixed single and plural, everything else looks good.

    Yeah, I have not done it yet. :smile:

    Are we not concerned about breaking backwards compatibility with Gadget based tools and scripts? We would also need to change the reader routines and so we would no longer be able to read 'Gadget' HDF5 files.

    That's a good point. There is no standardized Gadget-format beyond the very basic fields though. That would make us stay with plurals and capitalized first words. We can then extend that with plural CamelCaseFields.

  • The problem is that even the gadget case is inconsistent (PartType0/Coordinates v.s. PartType0/SmoothingLength v.s. PartType0/Masses v.s. PartType0/Density); it's the same problem as usual, unfortunately.

    I prefer the plurals as that makes it clear that they are arrays and not individual particles.

  • Other than that, I would want 'correct' camel case with no special characters and no spaces, for all fields. E.g. Maximal Temperature should be MaximalTemperature and Maximal Temperature scale-factor should be MaximalTemperatureScaleFactor. We can also change things to be more clear, e.g. sSFR becomes SpecificStarFormationRate.

  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
Please register or sign in to reply
Loading