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

BOV lacks very basic error checking #2792

Closed
shaomeng opened this issue Jul 12, 2021 · 5 comments · Fixed by #2809
Closed

BOV lacks very basic error checking #2792

shaomeng opened this issue Jul 12, 2021 · 5 comments · Fixed by #2809
Assignees
Milestone

Comments

@shaomeng
Copy link
Collaborator

Describe the bug

  1. Open VAPOR, import BOV file /glade/p/cisl/vast/vapor/Bugs/BOV_Bugs/bug4.bov
  2. Create and enable a volume renderer, something gets rendered.

However, this shouldn't happen, because the BOV header claims a certain data size (128x512x256) and data format (double) but the data file is only half of the required size. This means, VAPOR read in twice the amount of data that the data file provides and rendered something. Not good...

@shaomeng shaomeng added the Bug label Jul 12, 2021
@sgpearse
Copy link
Collaborator

Not saying we can't do better, but Visit doesn't do this either. Some onus is on the user to get their header right.

From their documentation:

Page 11 to page 12 of Getting Data Into VisIt:

If the picture is not quite what you expected then you can fine-tune the values in the “.bov” file until you get the picture that you want to see. The most common cause of errors is failing to set the DATA_SIZE and DATA_FORMAT keywords to the right values for your data file.

@shaomeng
Copy link
Collaborator Author

Well, a sanity check based on grid resolution and data types is less than 10 lines of code.

I'd think it makes sense to improve the software quality of VAPOR even if VisIt fails to do so.

@sgpearse sgpearse added the High label Jul 13, 2021
@sgpearse sgpearse self-assigned this Jul 13, 2021
@sgpearse sgpearse added this to the 3_5_0 release milestone Jul 13, 2021
@sgpearse
Copy link
Collaborator

@shaomeng - I believe that Bx.bot is the correct size, so bug4.bov (specified with DOUBLE for its format) shouldn't throw an error. Either way, the checks for data files being too big or small are currently on the branch.

128x512x256 = 16,777,216
16,777,216 * 8 bytes per value = 134,217,728

ls -l Bx.bot
-rw-r--r-- 1 shaomeng vapor 134217728 Jul 12 13:31 Bx.bot

@shaomeng
Copy link
Collaborator Author

@shaomeng - I believe that Bx.bot is the correct size, so bug4.bov (specified with DOUBLE for its format) shouldn't throw an error. Either way, the checks for data files being too big or small are currently on the branch.

128x512x256 = 16,777,216
16,777,216 * 8 bytes per value = 134,217,728

ls -l Bx.bot
-rw-r--r-- 1 shaomeng vapor 134217728 Jul 12 13:31 Bx.bot

Thanks @sgpearse . Though this bug report is buggy, good to know that you have implemented error checking.

To be specific, did you restrict the error checking to check both "file being too big" and "file being too small" problems, or only the "file being too small" problem?

If the code wants to read N bytes, and the file has M bytes, where M > N, it shouldn't be an error. Such scenarios could happen when

  1. there's non-zero offset values specified in the BOV so that a user is only interested in the 2nd half of a volume;
  2. a user wants to only visualize the 1st half of a volume by deliberately setting a smaller Z dimension. This can be useful when the whole volume is too big.

Anyway, if you only detect "file being too small" problem, but not do anything about "file being too big," actually, you don't even need to detect "file being too big," then the code will be good.

@sgpearse
Copy link
Collaborator

File too big has been removed. File too small has always been there.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants