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

Fix re-redistribution #680

Merged
merged 6 commits into from
Aug 23, 2023
Merged

Fix re-redistribution #680

merged 6 commits into from
Aug 23, 2023

Conversation

marchdf
Copy link
Contributor

@marchdf marchdf commented Aug 17, 2023

No description provided.

@marchdf marchdf force-pushed the add-rere branch 10 times, most recently from 8827146 to 555e315 Compare August 22, 2023 14:24
Copy link
Contributor

@baperry2 baperry2 left a comment

Choose a reason for hiding this comment

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

Can you update the Docs to reflect the new re-re capability? https://amrex-combustion.github.io/PeleC/geometry/EB.html#re-redistribution

Also, I tried running the sod-4 case on Macbook and it fails with the following error:

[Level 0 step 1] Advanced 32768 cells
[Level 1 step 1] ADVANCE at time 0 with dt = 0.0003961660569
... Computing MOL source term at t^{n}
SIGILL Invalid, privileged, or ill-formed instruction
See Backtrace.0 file for details

(base) bperry-38649s:Sod bperry$ cat Backtrace.0
0: amrex::BLBackTrace::print_backtrace_info(__sFILE*) (in PeleC3d.gnu.ex) (AMReX_BLBackTrace.cpp:189)
1: amrex::BLBackTrace::handler(int) (in PeleC3d.gnu.ex) (AMReX_BLBackTrace.cpp:99)
2: _sigtramp (in libsystem_platform.dylib) + 56
4: amrex::EBFluxRegister::FineAdd(amrex::MFIter const&, std::__1::array<amrex::FArrayBox const*, 3ul> const&, double const*, double, amrex::FArrayBox const&, std::__1::array<amrex::FArrayBox const*, 3ul> const&, amrex::FArrayBox const&, amrex::RunOn) (in PeleC3d.gnu.ex) (AMReX_EBFluxRegister.cpp:149)
5: PeleC::update_flux_registers(double, amrex::MFIter const&, amrex::FabType const&, std::__1::array<amrex::FArrayBox const*, 3ul> const&, amrex::FArrayBox const&) (in PeleC3d.gnu.ex) (PeleC.cpp:0)
6: PeleC::getMOLSrcTerm(amrex::MultiFab const&, amrex::MultiFab&, double, double, double) (in PeleC3d.gnu.ex) (Diffusion.cpp:0)

@marchdf
Copy link
Contributor Author

marchdf commented Aug 22, 2023

Updated the docs, good catch.

I will look into sod-4... I could swear I was using debug just fine...

@baperry2
Copy link
Contributor

Hmm it seems to run fine for me in Debug but fails in regular mode. The most fun kind of issue to debug haha

@jrood-nrel
Copy link
Contributor

I'm worried that error is buffer overflowing into instruction pointer code and why it's saying bad instruction.

@jrood-nrel
Copy link
Contributor

What does ASAN say?

@marchdf
Copy link
Contributor Author

marchdf commented Aug 22, 2023

Documenting here for posterity:

What Bruce saw is a typical thing we see on OSX and clang every so often. We guard against this in cmake with maytrap https://github.com/AMReX-Combustion/PeleC/blob/development/CMake/SetAmrexCompileFlags.cmake#L8. This doesn't happen on llvm+linux. So it's hard to replicate... I commented out the trapping so we only do it when we actually want to (which should just be in debug mode).

@marchdf marchdf requested a review from baperry2 August 22, 2023 20:15
@marchdf marchdf marked this pull request as ready for review August 22, 2023 20:15
@marchdf
Copy link
Contributor Author

marchdf commented Aug 22, 2023

We are going to wait on AMReX-Codes/amrex#3506 before we merge this

@marchdf marchdf marked this pull request as draft August 22, 2023 21:11
Copy link
Contributor

@baperry2 baperry2 left a comment

Choose a reason for hiding this comment

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

Thanks for updating the documentation.

I can successfully run the test case with fpe trapping turned off and the results look reasonable.

@marchdf marchdf marked this pull request as ready for review August 23, 2023 14:06
@marchdf marchdf enabled auto-merge (squash) August 23, 2023 14:06
@marchdf
Copy link
Contributor Author

marchdf commented Aug 23, 2023

Pics of what we can do now:

EB-C8:

Screen Shot 2023-08-23 at 10 42 30

Sod + sphere:

Screen Shot 2023-08-23 at 10 45 53
Screen Shot 2023-08-23 at 10 46 32

@drummerdoc
Copy link
Contributor

Excellent! Getting better all the time!

@marchdf marchdf merged commit bae27e6 into development Aug 23, 2023
14 checks passed
@marchdf marchdf deleted the add-rere branch August 23, 2023 22:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants