have CarpetIOScalar output grid scalar directly rather than applying reductions to them

Issue #666 closed
Roland Haas created an issue

Carpet already assumes that these grid scalars have the same value on all processors. So reducing them does not generate any information that is not already present by just looking at a single value.

The attached patch outputs grid scalars (from the root processor) directly, this means that grid scalar output files can be used to output timeseries like grid scalars (which is what many are).

The attached parfile and data files are a modfied version of a QLM test. Notice how much nicer to read the scalar output .scalar.asc is then the 0D output ..asc.

Keyword:

Comments (13)

  1. Erik Schnetter
    • removed comment

    Several reductions modify the value, e.g. the L2 norm which always returns a non-negative value, or norms of complex numbers. Thus applying reductions to scalars makes sense. I would prefer not to have a special case for grid scalars in reductions. For example, a 1-element array and a scalar should yield the same value when reduced.

    On the other hand, improving the output format of CarpetIOASCII is always a good idea. I think you want: - Don't extend lower-dimensional quantities to 3 dimensions, adding (meaningless) columns - Don't output grid structure information for grid arrays and grid scalars, which have a trivial grid structure - Don't output # comment lines if they separate only a single line of data - Maybe: don't output coordinates for grid arrays and grid scalars, which don't have a natural coordinate system associated with them This should not be difficult (probably ten lines of code). Since it changes the output format, it would need to depend on a new parameter and could not be the default.

    Another approach could be: - Implement a new "reduction" called "origin" that yields the value at the origin

    [How do I classify this as "reject patch; back to discussion"?]

  2. Roland Haas reporter
    • removed comment

    Notice that I carefully said that no '''information''' was generated by reducing :-). You could always do the norm/abs etc. on the data in the output files since there is only a single scalar to consider anyway. Anyhow though, not having different behaviour for different object types is a valid concern (see below).

    Yes too all the output-format improvement suggestions. I would not say that grid arrays cannot have coordinates (the sphericalsurfaces certainly could have some), but for grid scalars there simply seem to be no coordiantes at all (it's a single number).

    [How do I classify this as "reject patch; back to discussion"?]

    See #664 :-) Nicely demonstrates the idea behind #664.

    It's funny that you would suggest going for a "fake" reduction. I had actually implemented that option first and decided it was too much of a hack :-).

    Attached please find my implementation. Note that it only works for grid scalars (and maybe one element grid arrays).

  3. Erik Schnetter
    • removed comment

    This implementation of the "origin" reduction doesn't pick the value at the origin; rather, it picks a random value, depending on the order of the reduction. It also doesn't save on any MPI communication. This implementation makes only sense if all values are the same (which they are for grid scalars), and in this case, it returns the same as the "average" reduction... Do you really want to add a new, fake reduction?

    I'll look into modifying the standard ASCII output format.

  4. Roland Haas reporter
    • removed comment

    I aggree with all statemnets. I don't think that I can remove the MPI communication without changing more lines in CarpetReduce (essentially making the "origin" reduction a special case). This is one of the reasons why I did not propose it initially :-) Right now one can simply abuse eg. the 'maximum' reduction to get timeseries output for scalars that are identical on all processors (also without saving on MPI communication, but since this happens at IO time, I was less concerned about and MPI call).

    No, I don't really want to add a fake reduction. Hence the originally proposed the change to how CarpetIOScalar works which admittedly was not that much better. :-)

    Yes, the best thing is likely to modify CarpetIOASCII. If you have time to look into it that would be highly appreciated. Please let me know if you need help to test/debug it (I could also give it a try but that will take a while).

  5. Erik Schnetter
    • removed comment

    Roland, I pushed a change that introduces a new compact format. Please have a look, and feel free to modify/adjust this format.

  6. Roland Haas reporter
    • removed comment

    Thank you. I only looked at the committed code and the test files so far. It looks very nice. Thank you. I will try it out during the day and propose changes. One thing that I noticed was that the testsuite uses Carpet timing data, which is likely not a good idea since it should change each time the code is run, shouldn't it? One other thing might be that it might be nice to extent IOUtil to support a compact option in the per-variable option string so that one can turn on compact format only for some variables and leave eg. 1d ASCII output in the old format for the benefit of old postprocessing scripts that use the old format. I have no comments on the actual data so far.

  7. Roland Haas reporter
    • changed status to resolved
    • removed comment

    CarpetIOASCII:compact_format can be used to achieve the desired effect. Ok to close.

  8. Log in to comment