search

fusion-energy / openmc-dagmc-wrapper

A Python package that extends OpenMC base classes to provide convenience features and standardized tallies when simulating DAGMC geometry with OpenMC.
https://openmc-dagmc-wrapper.readthedocs.io/
MIT License
7 stars 2 forks source link

Major refactoring: moving from one class to an openmc-like architecture #60

Closed RemDelaporteMathurin closed 2 years ago

RemDelaporteMathurin commented 2 years ago

Todo:

RemDelaporteMathurin commented 2 years ago

@Shimwell I think we are already uploading to codecov, just it doesn't seem to comment on the PR

shimwell commented 2 years ago

This is an epic refactoring Remi, thanks so much for this.

RemDelaporteMathurin commented 2 years ago

This is an epic refactoring Remi, thanks so much for this.

image

codecov[bot] commented 2 years ago

Codecov Report

Merging #60 (2b3fb37) into develop (513cea8) will decrease coverage by 8.57%. The diff coverage is 85.29%.

:exclamation: Current head 2b3fb37 differs from pull request most recent head e9f83d6. Consider uploading reports for the commit e9f83d6 to get more accurate results Impacted file tree graph

@@             Coverage Diff             @@
##           develop      #60      +/-   ##
===========================================
- Coverage    93.86%   85.29%   -8.58%     
===========================================
  Files            3        6       +3     
  Lines          522      510      -12     
===========================================
- Hits           490      435      -55     
- Misses          32       75      +43
Impacted Files Coverage Δ
openmc_dagmc_wrapper/Geometry.py 38.77% <38.77%> (ø)
openmc_dagmc_wrapper/utils.py 89.28% <89.28%> (ø)
openmc_dagmc_wrapper/Tally.py 89.36% <89.36%> (ø)
openmc_dagmc_wrapper/Materials.py 97.36% <97.36%> (ø)
openmc_dagmc_wrapper/Settings.py 100.00% <100.00%> (ø)
openmc_dagmc_wrapper/__init__.py 100.00% <100.00%> (ø)
... and 2 more

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update d1f0a32...e9f83d6. Read the comment docs.

RemDelaporteMathurin commented 2 years ago

@Shimwell I feel like we need to add a test for reflective planes before merging this ... what do you think?

shimwell commented 2 years ago

I wonder, perhaps we could do a 90 degree sector model with a full 360 degree ring source and check the tally is 1/4 of the normal result

RemDelaporteMathurin commented 2 years ago

I wonder, perhaps we could do a 90 degree sector model with a full 360 degree ring source and check the tally is 1/4 of the normal result

That could work yeah

shimwell commented 2 years ago

I think we have a issue with the 2d mesh for sector models, this is a 360 degree ring source on a sector model. The mesh aspect ratio ends up a little wrong. I guess the number of pixels is the same in each dimension even if the geometry is larger in some dimensions neutron_effective_dose_on_2D_mesh_xy ,

shimwell commented 2 years ago

The model used in the examples is a 90 degree sector model with a graveyard cell. However the graveyard can be removed using 

pip install remove-dagmc-tags 
remove-dagmc-tags -i dagmc.h5m -o=dagmc_no_grave_yard.h5m -t graveyard
RemDelaporteMathurin commented 2 years ago

I think we have a issue with the 2d mesh for sector models, this is a 360 degree ring source on a sector model. The mesh aspect ratio ends up a little wrong. I guess the number of pixels is the same in each dimension even if the geometry is larger in some dimensions neutron_effective_dose_on_2D_mesh_xy ,

I don't really understand what I'm looking at here. Maybe this can be fixed in another PR?

shimwell commented 2 years ago

tests pass locally over here