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

odw.Model #110

Closed RemDelaporteMathurin closed 2 years ago

RemDelaporteMathurin commented 2 years ago

As discussed in #107 this PR introduces a new odw.Model class inheriting from openmc.Model.

Since this class stores the geometry, materials, tallies and settings, it makes sense to do all the communication between them in there.

RemDelaporteMathurin commented 2 years ago

@shimwell maybe we should hold this off until the CI is back on? If I accidentally introduced some bugs here, it will be a nightmare to debug because we are completely blind

RemDelaporteMathurin commented 2 years ago

@shimwell I lost track, do we have a repo with the files for the test suite?

shimwell commented 2 years ago

Sorry I still don't have a repo with the test files

shimwell commented 2 years ago

Getting closer to having a repo with files to download

The last docker release was able to produce files from example 1

https://github.com/fusion-energy/fusion_neutronics_workflow/releases/tag/develop

shimwell commented 2 years ago

The h5m files are now being downloaded by the test scripts and the tests are running again. Many are failing but this is something we can work with

RemDelaporteMathurin commented 2 years ago

The h5m files are now being downloaded by the test scripts and the tests are running again. Many are failing but this is something we can work with

Maybe we can work at fixing these tests in develop before merging this. We should try and stop merging PRs when tests don't work :(

shimwell commented 2 years ago

Yes sure, I meant work on this PR to fix things up.

shimwell commented 2 years ago

Down to 18 failing tests :chart_with_downwards_trend:

lots of the failing tests are for the same reason

AttributeError: 'RegularMesh2D' object has no attribute 'mesh_resolution'
RemDelaporteMathurin commented 2 years ago

Ah yes mesh_resolution is now resolution cause RegularMesh2D inherits from openmc.RegularMesh

Maybe this should be fixed in a seperate PR. I'll try to find some time to do it

shimwell commented 2 years ago

@RemDelaporteMathurin I think I've just fixed that bug. Down to just 4 failing tests now

shimwell commented 2 years ago

I'm hoping to get this passing all the tests and merged into develop today. Then perhaps we can take a step back and see what parts (if any) of this code are actually useful

codecov[bot] commented 2 years ago

Codecov Report

Merging #110 (1b83d44) into develop (77958d8) will decrease coverage by 5.51%. The diff coverage is 83.61%.

:exclamation: Current head 1b83d44 differs from pull request most recent head 0c3f8bc. Consider uploading reports for the commit 0c3f8bc to get more accurate results

Impacted file tree graph

@@             Coverage Diff             @@
##           develop     #110      +/-   ##
===========================================
- Coverage    87.89%   82.38%   -5.52%     
===========================================
  Files            6       14       +8     
  Lines          314      352      +38     
===========================================
+ Hits           276      290      +14     
- Misses          38       62      +24     
Impacted Files Coverage Δ
...c_wrapper/tallies/mesh_tallies/tet_mesh_tallies.py 40.00% <40.00%> (ø)
openmc_dagmc_wrapper/model.py 50.00% <50.00%> (ø)
openmc_dagmc_wrapper/Materials.py 65.71% <66.66%> (-34.29%) :arrow_down:
openmc_dagmc_wrapper/Geometry.py 72.72% <80.00%> (-7.28%) :arrow_down:
openmc_dagmc_wrapper/mesh.py 81.08% <81.08%> (ø)
openmc_dagmc_wrapper/tallies/cell_tally.py 95.23% <95.23%> (ø)
openmc_dagmc_wrapper/Tally.py 100.00% <100.00%> (+11.11%) :arrow_up:
openmc_dagmc_wrapper/__init__.py 100.00% <100.00%> (ø)
openmc_dagmc_wrapper/tallies/__init__.py 100.00% <100.00%> (ø)
...nmc_dagmc_wrapper/tallies/mesh_tallies/__init__.py 100.00% <100.00%> (ø)
... and 6 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 bf0a185...0c3f8bc. Read the comment docs.

shimwell commented 2 years ago

@RemDelaporteMathurin the tests are now passing I think this can be merged into the develop branch