-
Notifications
You must be signed in to change notification settings - Fork 105
Remove redundant timestep code #836
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
base: master
Are you sure you want to change the base?
Remove redundant timestep code #836
Conversation
…routine and condense.
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #836 +/- ##
==========================================
+ Coverage 43.24% 43.58% +0.33%
==========================================
Files 68 68
Lines 20183 19691 -492
Branches 2402 2359 -43
==========================================
- Hits 8728 8582 -146
+ Misses 9986 9669 -317
+ Partials 1469 1440 -29 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
… the failyre on NHVPC
…n issue for the intel compiler. I also have split out the loops in the subroutine to test if trying to block those together is causing an issue
… separate my for loops. Definately not desired behavior for ACC, but simple enough to work around.
Instead of |
3899ac5
to
272ccc4
Compare
Reverting back a commit since it looks like that small change broke almost every test case |
d178ffc
to
75d69a2
Compare
@danieljvickers, this is getting a little silly/comedic. Why don't you move on to a different issue that you feel you're suited for (send me a message of what you think it is, perhaps), and I'll think about this one a bit. This is really an edge case that probably needs a little more thought from our previous snafus with different types of runners (though this one seems especially special AKA stupid)... You're in a good place to create some containers for MFC. We could try creating those in a few different formats. |
Description
I noticed about 300 lines of extremely similar code related to how the RK steps were implemented. I instead created a single subroutine and removed the redundant copies. I also merged two of the parallel loops into one, as there was no race condition created by the two loops
Type of change
Please delete options that are not relevant.
Scope
If you cannot check the above box, please split your PR into multiple PRs that each have a common goal.
How Has This Been Tested?
Please describe the tests that you ran to verify your changes.
Provide instructions so we can reproduce.
Please also list any relevant details for your test configuration
mfc.sh
Test Configuration:
Ran on my local machine
Checklist
docs/
)examples/
that demonstrate my new feature performing as expected.They run to completion and demonstrate "interesting physics"
./mfc.sh format
before committing my codeIf your code changes any code source files (anything in
src/simulation
)To make sure the code is performing as expected on GPU devices, I have:
nvtx
ranges so that they can be identified in profiles./mfc.sh run XXXX --gpu -t simulation --nsys
, and have attached the output file (.nsys-rep
) and plain text results to this PR./mfc.sh run XXXX --gpu -t simulation --omniperf
, and have attached the output file and plain text results to this PR.