#1853: diagonal output in CarpetIOHDF5 disabled ---------------------+------------------------------------------------------ Reporter: rhaas | Owner: eschnett Type: defect | Status: new Priority: minor | Milestone: Component: Carpet | Version: development version Resolution: | Keywords: ---------------------+------------------------------------------------------

Comment (by rhaas):

There is a commit https://bitbucket.org/eschnett/carpet/commits/37f5f2e6ba973f1d8d5b1ddf194e84... that puts an assert(0) into the branch of an if statement handling diagonal output (https://bitbucket.org/eschnett/carpet/commits/37f5f2e6ba973f1d8d5b1ddf194e84...). You may want to check how "recent" your code is.

#1853: diagonal output in CarpetIOHDF5 disabled ---------------------+------------------------------------------------------ Reporter: rhaas | Owner: eschnett Type: defect | Status: new Priority: minor | Milestone: ET_2016_05 Component: Carpet | Version: development version Resolution: | Keywords: ---------------------+------------------------------------------------------ Changes (by rhaas):

* milestone: => ET_2016_05

Comment:

This must be addressed before the release otherwise diagonal hdf5 output will not work anymore.

#1853: diagonal output in CarpetIOHDF5 disabled ---------------------+------------------------------------------------------ Reporter: rhaas | Owner: eschnett Type: defect | Status: new Priority: minor | Milestone: ET_2016_05 Component: Carpet | Version: development version Resolution: | Keywords: ---------------------+------------------------------------------------------

Comment (by rhaas):

It was collateral to a change that significantly reduces memory requirements for 1d,2d output in HDF5 (and likely would also in ASCII output from Carpet). The previous implementation sends the full 3d data to a single process which then extracts the lower-d data and writes it to disk. For sufficiently large grids this is very slow (significant fraction of runtime) and runs out of memory. The breaking patch fixes the issue be doing the dimensional reduction before sending data. For diagonal data the reduction is not as straightforward and was not immediately implemented.

#1853: diagonal output in CarpetIOHDF5 disabled ---------------------+------------------------------------------------------ Reporter: rhaas | Owner: eschnett Type: defect | Status: new Priority: minor | Milestone: ET_2016_05 Component: Carpet | Version: development version Resolution: | Keywords: ---------------------+------------------------------------------------------

Comment (by hinder):

I am unhappy about removing a feature like this. Diagonal HDF5 output is useful in some situations (I have used it for black hole lattices, in which the diagonal is a special line on which we are very interested in the data). It is not very well supported; most of the metadata is missing unfortunately. I am in favour of reverting the patch and waiting for it to be implemented in a way that allows diagonal output to be maintained.

#1853: diagonal output in CarpetIOHDF5 disabled ---------------------+------------------------------------------------------ Reporter: rhaas | Owner: eschnett Type: defect | Status: new Priority: minor | Milestone: ET_2016_05 Component: Carpet | Version: development version Resolution: | Keywords: ---------------------+------------------------------------------------------

Comment (by knarf):

Replying to [comment:9 eschnett]:

The choices here are either "no diagonal output" or "runs out of memory"

(see above).

In all simulation sizes? I regularly do simulations that use lower- dimensional hdf5 output and haven't seen a memory problem there. Maybe my grid sizes are not "large enough" to trigger this, so then: which sizes are?

#1853: diagonal output in CarpetIOHDF5 disabled ---------------------+------------------------------------------------------ Reporter: rhaas | Owner: eschnett Type: defect | Status: new Priority: minor | Milestone: ET_2016_05 Component: Carpet | Version: development version Resolution: | Keywords: ---------------------+------------------------------------------------------

Comment (by rhaas):

Just piping up: as far as I remember, the simulation that failed is one that had a single rather large refinement level (only data from one reflevel is copied), as often happens during supernova simulations. The memory reduction in that case is quite large of course since one goes from 3d data to 2d (or even 1d) data.

Having said that, I have to admit that I am actually more in favor of reverting the change for the release version. I favor this option because current master fails to deliver promised functionality for all users while the version before the patch failed to promise functionality only for some users that know why this happens and how to avoid it (and the users to whom this originally happened are running off master anyway and not off the release branch). So my favored solution (given that I do not see a "proper" handling of diagonal output to be implemented, tested and trusted very quickly), would be to remove the patch the reduces memory consumption at the cost of disabling diagonal output for the release branch only.

My understanding of "development happens on master" and its relationship with release branches is that master may always break anything but that release branches may only break things after a one release announcement that this will happen.

#1853: diagonal output in CarpetIOHDF5 disabled ---------------------+------------------------------------------------------ Reporter: rhaas | Owner: eschnett Type: defect | Status: confirmed Priority: minor | Milestone: ET_2016_05 Component: Carpet | Version: development version Resolution: | Keywords: ---------------------+------------------------------------------------------ Changes (by hinder):

* status: new => confirmed

Comment:

In the call, we decided to revert this in master before the release. It was suggested that a working (though inelegant) solution may not be hard to implement, and this could happen after the release. I was not involved in this code, and I couldn't find a clear set of commits to revert. It seemed that the change was spread across more than one commit, and that one of the commits also changed other things. I think this would be better handled by someone who is more familiar with the changes that were made (i.e. Erik or Roland).