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

Broken ParaView export for DiffusionGrid #285

Open
TobiasDuswald opened this issue Oct 13, 2022 · 4 comments
Open

Broken ParaView export for DiffusionGrid #285

TobiasDuswald opened this issue Oct 13, 2022 · 4 comments
Assignees
Labels
bug help wanted ready Ready for merge / review solved

Comments

@TobiasDuswald
Copy link
Contributor

TobiasDuswald commented Oct 13, 2022

Description

Exporting the DiffusionGrid to ParaView leads to unexpected behavior, e.g. I see empty slices, sometimes filled with more or less random data. See images (yet to follow).

Work Log

  • Identified that the ParaView export works fine for certain combinations of DiffusionGrid::resolution and OMP_NUM_THREADS. For others it did not work as expected. E.g. resolution = 20 did work well for 4 threads but did not work for 8 threads.
  • @Senui observed the same problem and verified on macOS i386
  • The current setup always exports a certain number of slices 1 .. N, the error always appears for the first layer of slide N.
  • Double checked the entire export pipe line could not find an error.
  • Test case resolution = 20 with 4 and 8 threads. 4 threads export 4 * 5 slices. 8 threads export (5*3 + 5), e.g. the last slice should be equivalent. The diff tool shows no difference between the slice N=4 and N=6 for 4 and 8 threads respectively.
  • Restructured export in a hopefully more load balanced way using all threads if possible, ⭐ current believe: fixes the problem (:zap: Test fail, not sure if important or why)

To Reproduce

Simply have a diffusion grid and export to PV. Choose resolution%num_threads!=0.
Reproducer to follow.

Expected behavior

No empty space.

Setup (please complete the following information):

  • OS: macOS (i386, arm64); Linux (not clear yet)
  • BioDynaMo version v1.05.0-137fdb15
@TobiasDuswald
Copy link
Contributor Author

TobiasDuswald commented Oct 13, 2022

Often observed:
image

Expected
image

@TobiasDuswald
Copy link
Contributor Author

Reproducer
diffusion.zip

Reproducer is written for resolution 20, if you have 8 threads, you should see strange behavior. Using the branch pv-export should hopefully solve this problem

@TobiasDuswald
Copy link
Contributor Author

TobiasDuswald commented Oct 13, 2022

💻 Task for assignees

  • Verify bug
  • Stress test solution
  • Next steps: address failing ParaView tests and Performance

@TobiasDuswald
Copy link
Contributor Author

discussion with @LukasBreitwieser

  • Bug reproducible on Ubuntu 20.04
  • Fix works (visually) on Ubuntu 20.04
  • Unit tests actually pass; not on macOS
  • Explained test

Conclusion: fix tests, hopefully good to go

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug help wanted ready Ready for merge / review solved
Projects
None yet
Development

No branches or pull requests

6 participants