Adding tag to json_data for memory correction

keefemitman commented 2 years ago

Adding a tag to json_data for memory correction. But sxs.waveforms.rpxmb.save() is set to always raise an error?

moble commented 2 years ago

Currently, this PR would produce JSON data that (ignoring checks to see if the attributes exist) essentially looks like

            "transformations": {
                "truncation": L2norm_fractional_tolerance,
                "boost_velocity": w.boost_velocity.tolist(),
                "space_translation": w.space_translation.tolist()
            },
            "memory_correction": {
                "integration_time": w.memory_integration_start_time
            },

The transformations group is actually intended to record all the transformations — not just BMS transformations. Maybe modifications would be a better name for that group. Anyway, the memory correction info should probably go in there.

But this makes me think it might actually be better to be more explicit about these arguments, by including the name of the function doing the modification, and then its arguments as kwargs. So it would look more like this:

            "transformations": {
                "truncate": {"tol": L2norm_fractional_tolerance},
                "transform": {
                    "boost_velocity": w.boost_velocity.tolist(),
                    "space_translation": w.space_translation.tolist()
                },
                "add_memory": {"integration_start_time": w.memory_integration_start_time}
            },

(and maybe change transformations to modifications or something). This could break any scripts that read boost_velocity and space_translation from the mini catalog, but I can't imagine there are enough such scripts to worry about.

Any opinions?

keefemitman commented 2 years ago

Currently, this PR would produce JSON data that (ignoring checks to see if the attributes exist) essentially looks like
            "transformations": {
                "truncation": L2norm_fractional_tolerance,
                "boost_velocity": w.boost_velocity.tolist(),
                "space_translation": w.space_translation.tolist()
            },
            "memory_correction": {
                "integration_time": w.memory_integration_start_time
            },
The transformations group is actually intended to record all the transformations — not just BMS transformations. Maybe modifications would be a better name for that group. Anyway, the memory correction info should probably go in there.

But this makes me think it might actually be better to be more explicit about these arguments, by including the name of the function doing the modification, and then its arguments as kwargs. So it would look more like this:
            "transformations": {
                "truncate": {"tol": L2norm_fractional_tolerance},
                "transform": {
                    "boost_velocity": w.boost_velocity.tolist(),
                    "space_translation": w.space_translation.tolist()
                },
                "add_memory": {"integration_start_time": w.memory_integration_start_time}
            },
(and maybe change transformations to modifications or something). This could break any scripts that read boost_velocity and space_translation from the mini catalog, but I can't imagine there are enough such scripts to worry about.

Any opinions?

Yeah I like the idea of referencing the functions that do the modification, but then I think we should change transformations to modifications

sxs-collaboration / sxs

Adding tag to json_data for memory correction #54