Modify

Opened 5 years ago

Last modified 5 years ago

#1609 reopened defect

CarpetIOHDF5 incorrectly recovers grid scalars and arrays with DISTRIB=CONSTANT

Reported by: Roland Haas Owned by: Erik Schnetter
Priority: minor Milestone:
Component: Carpet Version: development version
Keywords: Cc:

Description

Due to the way that grid variables with DISTRIB=constant are handled, a recent change to Carpet (which is required to run on larger core counts) caused these variables to not be correctly recovered (the change was to add a new dimension rather than extend an existing one).

There were two things missing: grid scalars are handled as zero dimensional arrays in the flesh but as 1d arrays with 1 element in the driver, restoring variables (arrays or scalars) did not take the new way of handling DISTRIB=constant into account.

Since I have already committed fixes before and now have fixes for the fixes (sorry), it would be good if someone was to review them.

The typical failure mode is that grid scalars/arrays are not fully recovered which in the case of grid scalars is silent (since the whole variable is missing which we silently ignore) leaving them with undefined values.

Attachments (3)

0001-CarpetIOHDF5-fix-handling-of-DISTRIB-constant-arrays.patch (2.4 KB) - added by Roland Haas 5 years ago.
0002-Carpet-fix-handling-of-scalars-after-3fe72d9.patch (1.9 KB) - added by Roland Haas 5 years ago.
cp.par (1.7 KB) - added by Roland Haas 5 years ago.

Download all attachments as: .zip

Change History (10)

comment:1 Changed 5 years ago by Roland Haas

Status: newreview

comment:2 Changed 5 years ago by Erik Schnetter

Status: reviewreviewed_ok

Do you have test cases?

comment:3 Changed 5 years ago by Roland Haas

A test case turned out not that straightforward since the incorrect data would only ever appear on processes other than process 0 so Carpet would never write it do disk. Also contrary to what I originally stated, grid SCALARS are indeed still read correctly (since special code for group.dim==0 was taken out and thus they actually behave as before), for grid arrays however the effect is present. Attached is a demo parfile cp.par. To demonstrate do:

mpirun -n 2 cactus_bns_all -roe cp.par >cp.log
mpirun -n 2 cactus_bns_all -roe cp.par >rec.log

and notice that while rec.log lists

INFO (CarpetIOHDF5):   reading 'CARPETREGRID2::radius[0]' from dataset 'CARPETREGRID2::radius[0] it=0 tl=0'

this entry is missing from CCTK_Proc1.log.

With the patches CCTK_Proc1.log als contains this line, ie the grid array is successfully read on process 1.

I don't know how to make a test case that the test system would support out of this.

Changed 5 years ago by Roland Haas

Attachment: cp.par added

comment:4 Changed 5 years ago by Roland Haas

Resolution: fixed
Status: reviewed_okclosed

Applied as hash 56d5d70785357eb18c158cd37f693c75a20f5125 and 4357322ccee0497e2d11fd35165b5029b37b7650 of CarpetIOHDF5 and Carpet.

comment:5 Changed 5 years ago by Erik Schnetter

Resolution: fixed
Status: closedreopened

You could write a non-trivial value to the grid array, and after recovery, calculate both min and max via a local reduction, and output this via IOASCII.

comment:6 Changed 5 years ago by Frank Löffler

Is this still considered to be a blocker for the release?

comment:7 Changed 5 years ago by Roland Haas

Milestone: ET_2014_05
Priority: blockerminor

No, the bug has been fixed. The ticket was re-opened to provide a test case.

Modify Ticket

Change Properties
Set your email in Preferences
Action
as reopened The owner will remain Erik Schnetter.
Next status will be 'review'.
as The resolution will be set.
to The owner will be changed from Erik Schnetter to the specified user.
The owner will be changed from Erik Schnetter to anonymous.

Add Comment


E-mail address and name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.