Skip to content

Conversation

@rodriguez-jay
Copy link

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

@rodriguez-jay
Copy link
Author

rodriguez-jay commented Oct 29, 2025

@andrsd @wdhawkins

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.
Copy link
Collaborator

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.

Comment on lines +417 to +418
for (py::handle name : bndry_names)
bndrys.push_back(name.cast<std::string>());
Copy link
Collaborator

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.

Comment on lines +414 to +415
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;
Copy link
Collaborator

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.

Comment on lines +452 to +453
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;
Copy link
Collaborator

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);
Copy link
Collaborator

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;
Copy link
Collaborator

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();
Copy link
Collaborator

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
Copy link
Collaborator

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
Copy link
Collaborator

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;
Copy link
Collaborator

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)
Copy link
Collaborator

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,
Copy link
Collaborator

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;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Remove if not needed...

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.

3 participants