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

Eyoo/multicuts #1

Open
wants to merge 39 commits into
base: test-pr
Choose a base branch
from
Open

Eyoo/multicuts #1

wants to merge 39 commits into from

Conversation

ejyoo921
Copy link

@ejyoo921 ejyoo921 commented Jul 2, 2024

Summary

Additional background

Checklist

The proposed changes:

  • fix a bug or incorrect behavior in AMReX
  • add new capabilities to AMReX
  • changes answers in the test suite to more than roundoff level
  • are likely to significantly affect the results of downstream AMReX users
  • include documentation in the code and/or rst files, if appropriate

ejyoo921 and others added 4 commits June 28, 2024 15:08
…d a feature to capture the location of multicuts. EB/AMReX_EB2_3D_C.cpp will be the main file to edit since the function 'build_faces' is here.
…h dx, dy, and dz: This is given so that one can consider refining grid to avoid multicuts
@baperry2
Copy link
Owner

baperry2 commented Jul 3, 2024

Suggestions based on our chat today:

  • Add parmparse option that turns on/off visualization/printing of multicuts
  • When visualizing/printing multicuts, move the amrex::Abort outside of the MFIter, such that all multicuts in all boxes are captured (somewhere around here:
    ParallelAllReduce::Sum<int>({nsmallcells,nmulticuts}, ParallelContext::CommunicatorSub());
    )
  • Print out multicut x/y/z or i/j/k location instead of (or in addition to) making a plt file
  • If continuing to plot multicuts, refactor so all multicuts are plotted in a single plt file for each type of face (where the data is the number of cuts). This would mean creating a multifab before the MFIter loop starts in AMReX_EB2_Level.H and then passing additional Array4s into build_faces, then saving after the MFIter loop ends.
  • Add visualization/printing pathway for both 2D and 3D code

…lt and you may choose 'true' in your input file if you want to see the multicut locations.

-> eb2.plt_multiple_cuts = 'true'
2) If (plt_multiple_cuts), we delay aborting the code until finishing up the loop, including the build_cells part.
3) I just made the int total_multicuts to get the final number of multicuts -- to make sure in case it is not the same number as nmulticuts.
@ejyoo921 ejyoo921 closed this Jul 12, 2024
@ejyoo921 ejyoo921 reopened this Jul 12, 2024
@ejyoo921
Copy link
Author

sorry I accidentally clicked closing the pull request... 🤦🏻‍♀️

@ejyoo921
Copy link
Author

ejyoo921 commented Jul 12, 2024

Somethings done

  • Add parmparse option that turns on/off visualization/printing of multicuts
  • When visualizing/printing multicuts, move the amrex::Abort outside of the MFIter, such that all multicuts in all boxes are captured (somewhere around here:
    ParallelAllReduce::Sum<int>({nsmallcells,nmulticuts}, ParallelContext::CommunicatorSub());

Src/EB/AMReX_EB2_Level.H Outdated Show resolved Hide resolved
Src/EB/AMReX_EB2_Level.H Outdated Show resolved Hide resolved
…multicuts. We can now print out the index of multicut boxes but this current output seems to be no good use. Need to adjust more.
…ticuts.0 that includes physical location of the multicut cells: The coordinates indicate the cell center.
@ejyoo921
Copy link
Author

  • Print out multicut x/y/z or i/j/k location instead of (or in addition to) making a plt file
    loc_multicuts.txt

…H. Next step would be to create a multifab to store locations of the multicut in build_faces function and bring it out to AMReX_EB2_Leve.H. Also, try to output one plot file.
2) This is used as an input for build_faces.
3) Simply set value - multicut_arr(i,j,k) = 10.0 -  whenever we detect multicut.
4) In AMReX_EB2_Level.H (line 494 - ), we first check if multicut_arr stored the multicut locations (value 10.0).
 - this will be removed once we get right plot file.
5) The hope is that we get "volume" of multicut location when we read the plt file.
6) We probably do not need anything in this line, except the line 508 (WriteSingleLevelPlotfile).
 - The question is: Is this enought to see the multicuts?
@ejyoo921
Copy link
Author

Screenshot 2024-07-29 at 9 04 26 AM Screenshot 2024-07-29 at 9 04 41 AM Screenshot 2024-07-29 at 9 04 56 AM

Looking at "Volume" for multicut.
Adjusted "Scalar Opacity Unit Distance" to highlight the multicut locations.
See the bunny.

Src/EB/AMReX_EB2_Level.H Outdated Show resolved Hide resolved
Src/EB/AMReX_EB2_3D_C.cpp Outdated Show resolved Hide resolved
@@ -476,6 +490,12 @@ GShopLevel<G>::define_fine (G const& gshop, const Geometry& geom,
nsmallcells += nsm;
nmulticuts += nmc;
}
// EY: Here -> make plot file
if (plt_multiple_cuts)
Copy link
Owner

Choose a reason for hiding this comment

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

Move after the ParallelAllReduce for nmulticuts, then add nmulticuts > 0 && !cover_multiple_cuts to the conditional

Src/EB/AMReX_EB2_3D_C.cpp Outdated Show resolved Hide resolved
if (plt_multiple_cuts){
multicut_arr(i,j,k) = 10.0;
// Not GPU friendly
amrex::PrintToFile("loc_multicuts") << "fz: (x,y,z) = (" << problo[0]+(i)*dx[0] << ","<< problo[1]+(j)*dx[1] << "," << problo[2]+(k)*dx[2] << ") \n";
Copy link
Owner

Choose a reason for hiding this comment

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

surround the PrintToFile bit with an #ifndef AMREX_USE_GPU so this can still compile on GPU.

Copy link
Author

Choose a reason for hiding this comment

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

let me try to run things.

… xbx, ybx, and zbx: next step will be plotting face-center multifabs since they are multicut faces we are plotting now.
@ejyoo921
Copy link
Author

Screenshot 2024-08-15 at 3 50 50 PM I guess Visit won.

ejyoo921 and others added 4 commits August 26, 2024 17:39
…locations in using PrintToFile in AMReX_EB2_3D_C.cpp file. Also, most importantly, I fixed a stupid if-condition mistake in AMReX_EB2_Level.H: line 502. We should not abort the code when no multicuts present but plt_multiple_cuts = true
We now create separate plot files containing multi-cut locations from xbx, ybx, and zbx: next step will be plotting face-center multifabs since they are multicut faces we are plotting now.
…ile with data multicuts. See AMReX_EB2_3D_C.cpp
@ejyoo921
Copy link
Author

All the failings come from the same place.
/home/runner/work/amrex/amrex/Src/EB/AMReX_EB2_3D_C.cpp:474:22: error: embedding a directive within macro arguments has undefined behavior [-Werror,-Wembedded-directive] 474 | #ifndef AMREX_USE_GPU | ^

@baperry2
Copy link
Owner

This is the error I was looking for before!

A couple options:

  • simply remove the print statements as all the info should be accessible via the plot files
  • Dig in and see if you can find a place in Pele, AMReX, etc that do something similar and how they do it. I feel like there must be a way. Perhaps it would work to wrap the print in a function.

A next step is to see how this works for multilevel cases

…rintToFile, it seems like we don't need this condition for GPU

multicut_fcx.define(m_grids, m_dmap, 1, ng, mf_info);
multicut_fcy.define(m_grids, m_dmap, 1, ng, mf_info);
multicut_fcz.define(m_grids, m_dmap, 1, ng, mf_info);
Copy link
Owner

Choose a reason for hiding this comment

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

These should be defined similar to m_areafrac below, so that they have face data rather than cell-centered data

Copy link
Owner

@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.

Src/EB/AMReX_EB2_3D_C.cpp Outdated Show resolved Hide resolved
Src/EB/AMReX_EB2_2D_C.cpp Outdated Show resolved Hide resolved
Src/EB/AMReX_EB2_3D_C.cpp Outdated Show resolved Hide resolved
Src/EB/AMReX_EB2_3D_C.cpp Outdated Show resolved Hide resolved
Src/EB/AMReX_EB2_Level.H Outdated Show resolved Hide resolved
Src/EB/AMReX_EB2_Level.H Outdated Show resolved Hide resolved
Src/EB/AMReX_EB2_3D_C.cpp Outdated Show resolved Hide resolved
Merge branch 'eyoo/multicuts' of github.com:ejyoo921/amrex into eyoo/multicuts
@@ -310,6 +311,9 @@ int build_faces (Box const& bx, Array4<EBCellFlag> const& cell,
if (fy(i ,j+1,0) == Type::irregular) { ++ncuts; }
if (ncuts > 2) {
Gpu::Atomic::Add(dp,1);
if (plt_multiple_cuts){
Copy link
Owner

Choose a reason for hiding this comment

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

bring this outside of the if (ncuts > 2) scope. That way ncuts = 2 will show regular cut cells, ncuts > 2 multicuts, and ncuts = 0 cells that are either completely covered or completely fluid.

…(or very tiny) is covered, and some cell values > 2 means multicuts
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.

2 participants