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.

Sign up for GitHub

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

Jump to bottom

Fix parallel export to ParaView #286

Open
wants to merge 19 commits into
base: master
Choose a base branch
from
Open

Fix parallel export to ParaView #286

wants to merge 19 commits into from

Conversation

TobiasDuswald
Copy link
Contributor

Description in #285

@TobiasDuswald TobiasDuswald added bug WIP Work in progress ParaView labels Oct 14, 2022
@TobiasDuswald
Copy link
Contributor Author

TobiasDuswald commented Oct 28, 2022 •
edited
Loading

  1. Failing Sonar cloud is no issue here. PV test's currently not part of Coverage tracking
  2. Not clear if there's some stochasticity in the tests yet.
  3. For macOS the fix seems to work fine
  4. For Ubuntu the fix does not seem to work:
[  FAILED  ] FLAKY_ParaviewIntegrationTest.ExportDiffusionGrid_SlicesLtNumThreads
[2116](https://github.com/BioDynaMo/biodynamo/actions/runs/3345851088/jobs/5541905255#step:10:2117)
[  FAILED  ] FLAKY_ParaviewIntegrationTest.ExportDiffusionGridLoadWithoutPVSM
[2117](https://github.com/BioDynaMo/biodynamo/actions/runs/3345851088/jobs/5541905255#step:10:2118)
[  FAILED  ] FLAKY_ParaviewIntegrationTest.InsituDiffusionGrid_SlicesLtNumThreads

exits with exit code 136.

-> issue was related to OMP_NUM_THREADS=2, resolved in 9c59af2

@TobiasDuswald TobiasDuswald added the DoNotMerge Not ready for merge label Oct 28, 2022
@TobiasDuswald TobiasDuswald added ready-for-review PR is ready for review ready Ready for merge / review and removed WIP Work in progress DoNotMerge Not ready for merge ready-for-review PR is ready for review labels Oct 31, 2022
@TobiasDuswald
Copy link
Contributor Author

Ready to merge
Failing CI (Sonar) can be ignored here.

@TobiasDuswald TobiasDuswald self-assigned this Nov 1, 2022
nicogno
nicogno approved these changes Nov 23, 2022
View reviewed changes
Copy link
Contributor

@nicogno nicogno left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The fix works on my system and solves a major issue. Approved.

@sonarcloud
Copy link

sonarcloud bot commented Dec 19, 2022

SonarCloud Quality Gate failed.    Quality Gate failed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

0.0% 0.0% Coverage
0.0% 0.0% Duplication

@sonarcloud
Copy link

sonarcloud bot commented Apr 18, 2023

SonarCloud Quality Gate failed.    Quality Gate failed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 1 Code Smell

0.0% 0.0% Coverage
0.0% 0.0% Duplication

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@nicogno nicogno nicogno approved these changes

@LukasBreitwieser LukasBreitwieser Awaiting requested review from LukasBreitwieser

@FonsRademakers FonsRademakers Awaiting requested review from FonsRademakers

@vasvav vasvav Awaiting requested review from vasvav

Assignees

@nicogno nicogno

@Senui Senui

@LukasBreitwieser LukasBreitwieser

@TobiasDuswald TobiasDuswald

Labels
bug ParaView ready Ready for merge / review
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@TobiasDuswald @nicogno @Senui @LukasBreitwieser