Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Important invariant removed by assert #32

Open
NAThompson opened this issue Oct 28, 2019 · 4 comments
Open

Important invariant removed by assert #32

NAThompson opened this issue Oct 28, 2019 · 4 comments
Assignees
Labels
question Further information is requested

Comments

@NAThompson
Copy link
Contributor

If we were allowed to pass a zero tolerance to mgard_compress, then a very powerful unit test would be available.

However, this is removed by mgard_api.cpp, line 19, which asserts that tol >= 1e-8.

Is there a fundamental reason for this restriction? Note that if I remove it and set tol =0, then all returned data decompresses to zero. Is this expected behavior?

@ben-e-whitney ben-e-whitney added the question Further information is requested label Oct 28, 2019
@ben-e-whitney
Copy link
Collaborator

It appears that those assertions have been there since the beginning. Let me not speak for @tugluk, but I'd guess that they're just sanity checks. There isn't any mathematical reason we shouldn't allow a zero tol.

I misspoke earlier today – the structured code doesn't use an iterative linear solver, so the decomposition and recomposition routines should be 'exact' inverses. It would be a good test to have.

@ben-e-whitney
Copy link
Collaborator

@tugluk's comments on #31 and #33 made me realize I wasn't considering the quantizer at all. Without digging into the code, here's roughly what I think's going on:

  1. The original dataset u_nc (nodal values of u) is transformed to u_mc (multilevel coefficients).
  2. The 'importance' of multilevel coefficient u_mc[x] is something like 2 ^ sl * u_mc[x] where s is the smoothness parameter and l is the index of the mesh that introduced node x.
  3. So, if we want quantized_u_mc[x] (quantized coefficients) to be within τ of u_mc[x] 'overall', we need each quantized_mc[x] to be within τ / 2 ^ sl of u_mc[x] (ignoring that we also need to scale by the number of coefficients).
  4. Then the quantizer outputs an integer N such that quantized_u_mc[x] = N * τ / 2 ^ sl is as close as possible to u_mc[x].

If we want to allow a zero tol (or nonzero tol with s and l arbitrarily large), maybe we could just output the coefficients without quantizing? In the shorter term, we could possibly test the decomposition and recomposition functions alone, without quantizing and dequantizing.

@tugluk
Copy link
Collaborator

tugluk commented Oct 31, 2019

@ben-e-whitney, this is bang on. This is basically what I have in the code, L^infinity is a little different but the idea is the same.

Your suggestion is sure to work (obviously) with a loss in compression, using long ints in the quantizer is also an immediate intermediate solution of sorts.

@ben-e-whitney ben-e-whitney self-assigned this Sep 22, 2020
@ben-e-whitney
Copy link
Collaborator

ben-e-whitney commented Sep 22, 2020

The reimplemented compress function added in 6b43966 returns an object containing the compressed data along with the parameters passed to the function. I think we will probably add a member indicating which lossless compressor was used on the quantized multilevel coefficients. We could also use that member to indicate that the coefficients weren't quantized at all, and then we could support an error tolerance of zero.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
question Further information is requested
Projects
None yet
Development

No branches or pull requests

3 participants