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

Boundary conditions #56

Merged
merged 17 commits into from
Aug 16, 2024
Merged

Boundary conditions #56

merged 17 commits into from
Aug 16, 2024

Conversation

sammorrell
Copy link
Member

Implemented boundary conditions. Includes structures, supporting code and basic integration into MCRT code. Also includes some unit tests.

- Added structs for Boundary, BoundaryCondition, BoundaryDirection.
- Added handling of grid boundary hitting event.
- Added hard-coded default behaviour to all engines.
- Improvement of boundary condition structs to work with MCRT.
- Fixed bug where boundary hit finding would not be tolerant of rounding.
- Integration of boundary condition into engines and MCRT main.
- Added supporting ray-plane intersect function to support changes.
- Removed dependence of engine and travel upon output grid that stretches across whole domain. Making way for more flexible output in the future.
@sammorrell sammorrell requested a review from EdHone April 18, 2024 08:54
@sammorrell sammorrell self-assigned this Apr 18, 2024
Copy link
Collaborator

@EdHone EdHone left a comment

Choose a reason for hiding this comment

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

Some minor comments but otherwise this is a great change - the level of integration with the existing systems is great to see

src/bin/mcrt.rs Outdated Show resolved Hide resolved
src/geom/domain/boundary.rs Show resolved Hide resolved
Copy link
Collaborator

Choose a reason for hiding this comment

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

A test for this function would be nice

@EdHone
Copy link
Collaborator

EdHone commented Apr 25, 2024

Thanks for humoring my nitpicks! Merge away, Sam!

sammorrell and others added 13 commits August 1, 2024 16:05
- Started refactor of output with new `io` module.
- Added volume output + tests.
- Added plane output + tests.
It makes more sense for the `PhotonCollector` struct to be contained in
the output module, so I have moved it there. I have changed references in
the code to point to this new location.
This struct is intended as the new central object containing all of the
output code for `mcrt`. It contains all of the other outputs. I am in the
process of refactoring all of the output structs to work with this.

- Added `Output` struct.
- Added builder structs which support deserialisation from JSON to allow init from config.
- Added supporting structs around output types to deserialise / integrate.
- Output struct in `io` module now implemented.
- Added Builder structure to deserialise and build Output.
- Added supporting deserialisation, config and builder structs for output types.
- Added voxel distance calculation to output volume to support next steps.
- Added `OutputRegistry` to support output linking.
This now means that the output parameters can be read in as part of the
main JSON5 input for the MCRT software. I had added a redirect here, so
it can be included either in the same file, or separate. Also added
output section to display.
This function was previously using an invalid min finding iter-chain. It
also now sensibly returns f64::INFINITY instead of a None in the case of
distance to a voxel being None (no voxel boundaries in path of packet).
This commit adds new the new Output struct to the MCRT executable. Including
implementing new types throughout the code and integrating into existing
code.

Some more specific comments on changes:
- Modified engines and `travel` implementation to output into new objects.
- Modified `mcrt` exec to use new output object.
- Added extra implementations to clone output.
- Added placeholder implementations of `Save` for `Output`.
These grids were too large and wouldn't compile on a computer with less
RAM. I hadn't forgotten to change this back.
- Added saving to new `Output` struct.
- Added saving to output plane struct.
- Converted voxel distance measurement in engines to new system.
- Fixed bug where killed photons would continue to propagate in all engines.
- Added Display implementations for supporting structs.
- Fixed some warnings, mainly due to unneeded imports.
Removed the grid config and replaced with the boundary conditions. Many
changes to support these changes. Added structs and types to support deserialisation
and building. Integrated into the MCRT binary.

Note, this commit breaks the fluorescence engine, but I think we can work around this.
The removal of the `grid` parameter also removed access to a consistent
measurement grid over which to index into the shifts map in the fluorescence engine. However, I have engineered an alternate solution which indexes using
the bounds of the simulation and takes from the res of the grid, potentially
making this solution more flexible.
@sammorrell sammorrell merged commit 7a50a3d into develop Aug 16, 2024
1 check passed
@sammorrell sammorrell deleted the boundary-conditions branch August 16, 2024 08:04
@sammorrell sammorrell restored the boundary-conditions branch August 16, 2024 08:12
@sammorrell sammorrell deleted the boundary-conditions branch August 16, 2024 08:24
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