cabouman / mbircone

BSD 3-Clause "New" or "Revised" License
11 stars 9 forks source link

NaNs when using zero weights in shepp Logan demo #42

Closed dyang37 closed 3 years ago

dyang37 commented 3 years ago

Since the PR is already merged I'll open a separate issue for this.

I tested the previous PR that included the NaN fixes on a 3D shepp Logan example with zero weights, but still get NaNs:

************************** Iteration 0  (max. 20) **************************
*  Cost                   = 0.0000000000e+00
*  Rel. Update            = 0.0000000000e+00 % (threshold = 2.0000000000e-02 %)
*  RWFE = ||e||_W/||y||_W = -nan       % (threshold = 0.0000000000e+00 %)
*  RUFE = ||e|| / ||y||   = 1.0000000000e+02 % (threshold = 0.0000000000e+00 %)
* ----------------------------------------------------------------------------
*  1/M ||e||^2_W          = 0.0000000000e+00 = 1/inf
*  weightScaler_value     = 1.0000000000e+00 = 1/1.0000000000
* ----------------------------------------------------------------------------
*  voxelsPerSecond        = 0.0000000000e+00
*  time icd update        = 2.1457672119e-06 s
*  ratioUpdated           = 0.0000000000e+00 %
*  totalEquits            = 0.0000000000e+00
******************************************************************************

************************** Iteration 1  (max. 20) **************************
*  Cost                   = 0.0000000000e+00
*  Rel. Update            = 0.0000000000e+00 % (threshold = 2.0000000000e-02 %)
*  RWFE = ||e||_W/||y||_W = -nan       % (threshold = 0.0000000000e+00 %)
*  RUFE = ||e|| / ||y||   = 1.0000000000e+02 % (threshold = 0.0000000000e+00 %)
* ----------------------------------------------------------------------------
*  1/M ||e||^2_W          = 0.0000000000e+00 = 1/inf
*  weightScaler_value     = 1.0000000000e+00 = 1/1.0000000000
* ----------------------------------------------------------------------------
*  voxelsPerSecond        = 8.0133872599e+04
*  time icd update        = 1.7565979958e+00 s
*  ratioUpdated           = 1.0000000000e+02 %
*  totalEquits            = 1.0000000000e+00
******************************************************************************
cabouman commented 3 years ago

OK, good.

dyang37 commented 3 years ago

As a matter of fact I also get nans when using zero weights on Jordan's svmbir nanfix branch:

So maybe someone else could double check this just in case it's not a problem on my end.

dyang37 commented 3 years ago

Yes I think the merge of PR is fine. Since we did not break anything that was not broken before.

I think this is a common issue in both svmbir and mbircone, since I'm getting similar issue with nanfix branch of svmbir. I already commented under the svmbir issue regarding this problem.

dyang37 commented 3 years ago

Closing this since the branch is merged. For the NaNs produced by super voxel updates with lots of threads in svmbir, there's currently not a directly mapping to the icd update algorithm in mbircone. We may reopen this issue if we encounter such problem in mbircone.