Add option to label snapshots by the time

I can see why you would want to do it. But I am not a big fan of the current implementation.

If my time units are such that I get more than one snapshot between two integers rounding of e->time I will have a problem. Any cosmological run has e->time<1 for the length of the runs. So that's no good.

Ah good point! Should I just add some check that e.g. delta_time is > 1 to prevent obviously wrong attempts (but isn't fool-proof), or could do something like change the yml variable to snapshot:label_time_format that must be either 'i' or 'f' to format as an integer or a float, with '0' as the default to use the normal system?

That is not going to fundamentally solve the problem. How many digits will you use if you use 'f'? That will be problem (and units) dependent. That's also true for i. It just happens that in the case of your simulations, the time units you have can map more or less to a reasonable range of integers.

If you want to translate any valid time into something meaningful for snapshot labels you will need a more complex mechanism than that.

Right. How about using the yml to pass the actual string format you want?

  if (e->snapshot_label_time_int == 1)
    snprintf(fileName, FILENAME_BUFFER_SIZE, e->snapshot_label_format, baseName,
             (int) round(e->time));
  else if (e->snapshot_label_time_float == 1)
    snprintf(fileName, FILENAME_BUFFER_SIZE, e->snapshot_label_format, baseName,
             e->time);
  else
    snprintf(fileName, FILENAME_BUFFER_SIZE, "%s_%04i.hdf5", baseName,
             e->snapshot_output_count);

So the user can set e->snapshot_label_format = "%s_%06i.hdf5" or "%s_%3e.hdf5" etc. Does that work in C? Is there an easy way to change it to something that works like "%s_" + e->snapshot_label_format + ".hdf5" so the user input is just the "%06i"? I've not done strings in C much.

Or any different approach suggestions?

marked as a Work In Progress

changed the description

Since this is all optional I don't see any issue with adding it. Re: using dynamic formats. That is easy you just construct the format string on the fly and pass it into snprintf, but you'll need to change (int) round(e->time) to e->time for the general case and construct a format for the float, not an int. To truncate that is easy, just use %s_%.0f.hdf5, which will discard the remainder. Best to just specify the %.0f part in the parameter and construct the fuller format.

As for handling the general case of a floating value in a filename. Ouch, that could be nasty (as I also mentioned I think tools like gadgetviewer rely on these names as they are so keeping things as they, by default, is a good idea).

added 1 commit

1209dc00 - Edit the time label to be explicitly for integers

Compare with previous version

added 1 commit

16121703 - Edit the time label to be explicitly for integers

Compare with previous version

Thanks Peter! For now, I've made the parameter name more explicit and added a quick check (that isn't foolproof but will catch obvious mistakes) so that it can do what I want and is clearly a non-standard option that people won't often use.

I'd be happy to implement the dynamic format idea if we think anyone might use it, but my impression is that that's very unlikely. I figure while it's okay to have rarely used non-default options like this available, there's no point putting in more complicated ones that we don't expect to every be used so will just clutter the code.

unmarked as a Work In Progress

changed the description

I am fine with it since you are offering to make it optional. Can I ask to document the option in the RTD? See with @jborrow if you need help with this. We should start having all the options documented there.

This can be merged as soon as the stylistic changes are applied.

added 1 commit

41ee0e68 - Style improvements.

Compare with previous version

resolved all discussions

merged

mentioned in commit daa00858

Add option to label snapshots by the time

Merge request reports

Activity