-
Notifications
You must be signed in to change notification settings - Fork 34
Surface Response #810
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: main
Are you sure you want to change the base?
Surface Response #810
Conversation
rodriguez-jay
commented
Oct 24, 2025
- Added functionality to export surface angular fluxes at a specified boundary id or user provided surface id.
- Added functionality for computing the response function along a surface provided forward and adjoint surfaces.
|
In this draft PR I added functionality to write and read surface angular flux data. In order to readily handle the data I created a few structured objects in lbs_problem_io.h. Do you mind taking a look and provide some input on the implementation? Thanks! |
| Parameters | ||
| ---------- | ||
| file_base: str | ||
| File basename. |
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.
Please don't forget to document bndry_names and surfaces too.
If I understand correctly, surfaces is a pair (a.k.a tuple of size 2) of a str and a double. I think it would be more descriptive for user to have this argument as a tuple rather than a list.
Also, don't forget to thrown an error when py::len(surf_list) != 2. Otherwise, the code don't do anything and users won't be able to notice that they provide incorrect arguments.
| for (py::handle name : bndry_names) | ||
| bndrys.push_back(name.cast<std::string>()); |
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.
Please remember to throw exception if boundary name is not a supported one.
| std::map<std::string, uint64_t> allowed_bd_names = LBSProblem::supported_boundary_names; | ||
| std::map<std::uint64_t, std::string> allowed_bd_ids = LBSProblem::supported_boundary_ids; |
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.
replace this with const std::map<...>&. You don't need a copy of these maps to use them.
| std::map<std::string, std::uint64_t> supported_bd_names = LBSProblem::supported_boundary_names; | ||
| std::map<std::uint64_t, std::string> supported_bd_ids = LBSProblem::supported_boundary_ids; |
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.
please replace these with const std::map<...>&.
| for (py::handle name : bndry_names) | ||
| { | ||
| std::string bndry = name.cast<std::string>(); | ||
| // bndry_map[bndry] = supported_bd_names.at(bndry); |
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.
please remember to check for boundary. you should always assume that the user can input anything (misspelled names for example).
| H5ReadDataset1D<double>(file_id, group_name + "/values", values); | ||
| for (uint64_t c = 0; c < file_num_local_cells; ++c) | ||
| { | ||
| // bool isBndry = false; |
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.
remove if this is not needed
|
|
||
| log.Log() << "Writing surface angular flux to " << file_base; | ||
|
|
||
| std::vector<std::vector<double>>& psi = do_problem.GetPsiNewLocal(); |
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.
auto & psi = ...
| std::vector<std::string> bndry_tags; | ||
|
|
||
| // =========================================================== | ||
| // The goal is limit the number of itrations over all cells |
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.
itrations => iterations (typo)
| // =========================================================== | ||
| // The goal is limit the number of itrations over all cells | ||
| // Sweep through once and map cell_ids to boundaries then | ||
| // upack them into 1D vectors for exporting to H5 |
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.
H5 -> HDF5 (that is the name of the format. h5 is one of the extensions used...)
| unsigned int f = 0; | ||
| for (const auto& face : cell.faces) | ||
| { | ||
| bool isSurf = false; |
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.
"isSurf" -> "is_surf" (coding standard)
| LBSSolverIO::ReadSurfaceAngularFluxes( | ||
| DiscreteOrdinatesProblem& do_problem, | ||
| const std::string& file_base, | ||
| std::vector<std::string>& bndrys) |
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.
should this be const &?
| LBSSolverIO::WriteSurfaceAngularFluxes( | ||
| DiscreteOrdinatesProblem& do_problem, | ||
| const std::string& file_base, | ||
| std::vector<std::string>& bndrys, |
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.
Should this be const &?
| const auto& phi_dagger = buffer_data.first; | ||
| const auto& psi_dagger = buffer_data.second; | ||
| // const auto& phi_dagger = buffer_data.first; | ||
| // const auto& psi_dagger = buffer_data.second; |
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.
Remove if not needed...