Improved TokamakSource (with properties)

[LiamPattinson]

I've made a series of updates to TokamakSource to ensure it can't be set up with broken data, and cleaned up some TODO sections while I was at it.

The most significant change is the introduction of a utility file called properties.py, which implements a somewhat-generic 'property factory' and a few specific use cases. This allows you to do things like declare a class attribute must be a positive float, and protects against setting it to something inappropriate after initialisation. It might be a little overkill for these purposes, and it can instead be achieved by a simple list of checks within TokamakSource.__init__. I've mocked up both methods, and will submit the other version as a separate pull request. It's worth noting that I'm working on developing the property factory bits into an independent package, so I may be able to remove these files in a later build and simply replace them with a new dependency in setup.cfg.

I believe I've figured out what combination of geometry inputs are acceptable (major radius, minor radius, pedestal radius, Shafranov factor, triangularity, elongation), but I don't know what bounds should be set on the various ion densities and ion temperatures. It would be helpful if you could let me know any further constraints we can place on these values.

There were some sections marked TODO within TokamakSource, in which a list of ion densities/temperatures was built one element at a time in a for loop: https://github.com/fusion-energy/openmc-plasma-source/blob/ddef660343a211d1e9498a2cdcf38961ee3bdefc/openmc_plasma_source/tokamak_source.py#L112-L130 These have been rewritten to make use of np.where. As NumPy is a little better for reporting floating point issues than raw Python, I'm now seeing RuntimeWarnings for 'invalid value encountered in power'. It's hard to say if there's actually a cause for concern here, as I don't believe I've changed the overall logic anywhere. It would definitely be worth looking over those changes in detail to ensure I haven't broken anything.

Finally, I've updated test_tokamak_source.py extensively, adding unit tests to ensure that TokamakSource builds correctly when fed 'good' data and that it exits in a particular way when fed 'bad' data. I also changed how the hypothesis strategy was working as it was struggling to generate useful data after a few more constraints were added.

[shimwell]

This is really nice work, it will take me a little while to understand everything

[codecov[bot]]

Codecov Report

Merging #48 (8f2ccbb) into develop (7756a17) will increase coverage by 0.82%. The diff coverage is 100.00%.

:exclamation: Current head 8f2ccbb differs from pull request most recent head 7f9aa0d. Consider uploading reports for the commit 7f9aa0d to get more accurate results

@@             Coverage Diff             @@
##           develop      #48      +/-   ##
===========================================
+ Coverage    93.67%   94.50%   +0.82%     
===========================================
  Files            6        6              
  Lines          174      200      +26     
===========================================
+ Hits           163      189      +26     
  Misses          11       11              

Impacted Files	Coverage Δ
openmc_plasma_source/tokamak_source.py	98.33% <100.00%> (+0.46%)	:arrow_up:

---

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 7756a17...7f9aa0d. Read the comment docs.

[RemDelaporteMathurin]

@LiamPattinson there's a small drop in the diff coverage, do you think you can test these lines too?

[LiamPattinson]

@LiamPattinson there's a small drop in the diff coverage, do you think you can test these lines too?

I've added some extra tests to cover the new checks on TokamakSource functions, which should hopefully fix the coverage issue. I also moved the property bits to a new package that I'm (with apologies) calling proper_tea, which means I should be able to drop that functionality into Paramak fairly easily.

[shimwell]

proper_tea :rocket: