Open-Sn / opensn

open-source Sn software
https://open-sn.github.io/opensn/
Other
22 stars 18 forks source link

Reorganization of sweep utilities #125

Open zhardy-lanl opened 9 months ago

zhardy-lanl commented 9 months ago
wdhawkins commented 9 months ago

After looking at things, I vote that we create framework/sweep to contain everything currently in framework/mesh/sweep_utilities and the cbc stuff in modules/linear_boltzmann_solvers/b_discrete_ordinates_solver/sweepers. The organization of framework/sweep is a bit more up in the air, but I can see something like angle_set, group_set, schedulers, spds, sweep_chunk, and communicator (although I hate that term and will probably opt for something different at some point).

ragusa commented 9 months ago

Just do it!!

zhardy-lanl commented 9 months ago

Sorry for being late on this. Crazy day. From a macroscopic point of view, I think that anything within sweep_utilities that refers to the angular flux or the number of groups (or any LBS information) should live within the LBS solver. From a quick glance, it looks like SPDS and SPLS could stay in framework/mesh since they do not require LBS information, although I am not sure what the latter stands for off the top of my head. With this being said... I lean towards picking up most of framework/mesh/sweep_utilities and moving it to modules/linear_boltzmann_solver/b_discrete_ordinates_solver. Going through the mechanics of sweeping in the code is very confusing with most of the angular flux handling infrastructure within framework in my opinion.

wdhawkins commented 9 months ago

I think I would prefer to keep all of the sweep-related classes together. Although SPDS, SPLS, and FLUDS may not require explicit LBS information, they are specifically designed for the sweep. I'm not opposed to moving everything to modules/linear_boltzmann_solvers/b_discrete_ordinates_solver/sweep though. I'm curious... how far into framework does the angle-set, cell-set, group-set concept extend? Or does it at all?

SPDS -> Sweep Plane Data Structure SPLS -> SPDS Local Subgrid

wdhawkins commented 9 months ago

What are everyone's thoughts on the following renaming:

CBC -> CBCSweep AAH -> CSetSweep

ragusa commented 9 months ago

I prefer the option where all the sweep related files are in the same directory. So, I tend to preferframework/sweep. However, I am uncertain whether that single location should be in framework or not. Honestly, I am ok with it. Pushing even more things under b_discrete_ordinates_solver/ could be ok but I am afraid lbs is already super big so if they are things that can live comfortably outside of it, why not.

zachhardy commented 9 months ago

I would vote to keep everything with the discrete ordinates solver. Currently, it is the only thing that uses anything within sweep utilities. Separating it from the thing that does the sweeping in my opinion makes the code organization more muddy in my opinion, particularly when so much of sweep utilities refers to angular fluxes, groups, etc.

To the point of LBS getting big, I don't think anything in sweep utilities touches the Lua, so from the user perspective it doesn't change anything. There may be better strategies, but going into lbs is the most rationale first step in my opinion.

wdhawkins commented 9 months ago

I'll go with moving sweep into lbs and with the name change above. That's a solid first step, and we can refine from there.