-
Notifications
You must be signed in to change notification settings - Fork 68
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
New Example CI + Fixes to some broken example! #525
Conversation
I'm not exactly sure whats wrong with the test suite, but I think if I add the --generate flag they should be okay. Do we want the test suite to even test for examples because that would add another 30 minutes or more to them. Also I need to remove the -r flag from the example suites but it looks like it worked fine. |
@okBrian we can add this as a separate GitHub workflow (Examples-test.yml, Example Smoke Test). |
the |
@okBrian It's important that these cases run and produce output that isn't nonsense. So you can run the "example suite" with If |
If you specify Whether we should or should not use the |
…1 on examples, fixed m_patches typo. todo: fix 1D bubble cases
toolchain/mfc/test/test.py
Outdated
@@ -180,7 +181,7 @@ def _handle_case(case: TestCase, devices: typing.Set[int]): | |||
raise MFCException(f"Test {case}: {msg}") | |||
|
|||
if ARG("test_all"): | |||
case.delete_output() | |||
# case.delete_output() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
might want to put this back?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It breaks the 1D bubble cases. Not sure why maybe its deleting the D folder I'm creating.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
perhaps associated with current bug in CI that if it has to 'retry' a simulation, it will try to create a directory that already exists
I am expecting the code to not work with --no-mpi so thats another thing to debug... I think everything with mpi should pass hopefully. |
@okBrian you should be able to login to Frontier to see why your CI is failing. It is failing at build (link) time. It might be due to a change you made in the |
@okBrian, I also noticed that it failed in some cases with GNU and Intel compilers. This suggests that the flags might not be doing what they should (or you didn't add enough). One way to be sure you "have enough" is just using |
CMakeLists.txt
Outdated
@@ -186,13 +207,15 @@ elseif ((CMAKE_Fortran_COMPILER_ID STREQUAL "NVHPC") OR (CMAKE_Fortran_COMPILER_ | |||
if (CMAKE_BUILD_TYPE STREQUAL "Debug") | |||
add_compile_options( | |||
$<$<COMPILE_LANGUAGE:Fortran>:-O0> | |||
-C |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i'm not sure this syntax works. you could double check by looking at the cmake output of a specific run. i think you need to add
$<$<COMPILE_LANGUAGE:Fortran>:-<flag>>
on separate lines.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I will fix this in my next commit. Thanks
With the most recent commits these are the problematic examples Ubuntu MPI. no-Debug, True Ubuntu Mpi, Debug, False GT CPU Pheonix |
@okBrian can you visualize the output of a few of these cases (compared to the cases that are 'working')? I suspect that, given how diverse they are, some of them are just numerically unstable and eventually produce very large numbers that different compilers handle differently. |
Remaking PR |
Description
Added New Example CI, - Average runtime - 30 minutes
Why MacOS runner?
From my initial testing its much faster than the ubuntu images. Some examples ran over two times faster on the MacOS runner
Why 15625, (125, 125), or (25, 25, 25) for cell boundaries?
Some 3D examples break if m < 25, and this seems like a good spot to put examples so they don't take too long
Fixes #474
Type of change
Scope
How Has This Been Tested?
Test Configuration:
gcc 14, Ubuntu 22,04.4 LTS
Github Action Macos Runner
Checklist
./mfc.sh format
before committing my code