#797: N13CarpetRegrid211snap_coarseE does not hold after being enforced in cell centering ---------------------+------------------------------------------------------ Reporter: rhaas | Owner: eschnett Type: defect | Status: review Priority: major | Milestone: Component: Carpet | Version: Resolution: | Keywords: ---------------------+------------------------------------------------------ Changes (by rhaas):
* status: new => review
Comment:
The issue seems to be an explicit check in expand_for for an empty() *this. Since all the other manipulation functions also seem to contain at least BBBOX_ASSERTs on emptyness in at least some direction, simply removing the test seems like a bad idea. Right I am testing (successfully so far!) the following code, which is just the code pieces from the various bbbox member functions expanded out. {{{#!cpp #if(0) // this fails for very thin strips where cotracted_for creates a temporary empty box snapped |= bb. expand(reffact-1 - buffers % reffact). contracted_for(cbase). expanded_for(base). expand(buffers % reffact); #else // same math as above but avoid in particular expanded_for's set-to- empty ivect lower = bb.lower(); ivect upper = bb.upper(); ivect stride = bb.stride(); cout << "aa: " << ibbox(lower,upper,stride) << endl; // expand(reffact-1 - buffers % reffact) { lower -= (reffact-1 - buffers[0] % reffact) * stride; upper += (reffact-1 - buffers[1] % reffact) * stride; } cout << "aa.expand(): " << ibbox(lower,upper,stride) << endl; // contracted_for(cbase) { stride = cbase.stride(); ivect loff = ((lower - cbase.lower()) % stride + stride) % stride; ivect uoff = ((upper - cbase.lower()) % stride + stride) % stride; lower += (stride - loff) % stride; // go inwards upper -= uoff; } cout << "aa.expand().contracted_for(): " << ibbox(lower,upper,stride) << endl; // expanded_for(base) { stride = base.stride(); ivect loff = ((lower - base.lower()) % stride + stride) % stride; ivect uoff = ((upper - base.lower()) % stride + stride) % stride; lower -= loff; // go outwards upper += (stride - uoff) % stride; } cout << "aa.expand().contracted_for().expanded_for(): " << ibbox(lower,upper,stride) << endl; // expand(buffers % reffact); { lower -= (buffers[0] % reffact) * stride; upper += (buffers[1] % reffact) * stride; } cout << "aa.expand().contracted_for().expanded_for().expand(): " << ibbox(lower,upper,stride) << endl; snapped |= ibbox(lower,upper,stride); #endif }}}
Erik: I don't quite trust my understanding of the "reffact-1 - buffers % reffact" of the algorithm. Do you have an idea if it would be possible to rewrite the order of operations so that it never generates an empty box?
Otherwise the algorithm seems fine, ie. even for a very tiny strip it produces eg. a left edge that is a the same location as that of a thick strip with the same left edge initially.