NDE implementation

[htjb]

Description

Adding in optional neural density estimator plot functions as an alternative to KDE and histogram plotters.

Fixes #334

Checklist:

• I have performed a self-review of my own code
• My code is PEP8 compliant (flake8 anesthetic tests)
• My code contains compliant docstrings (pydocstyle --convention=numpy anesthetic)
• New and existing unit tests pass locally with my changes (python -m pytest)
• I have added tests that prove my fix is effective or that my feature works
• I have appropriately incremented the semantic version number in both README.rst and anesthetic/_version.py

[AdamOrmondroyd]

Just at a glance, you might want to have a look at how the optional dependency of fastkde is handled so you can do something similar with margarine

[codecov]

Codecov Report

Attention: Patch coverage is 11.11111% with 136 lines in your changes are missing coverage. Please review.

Project coverage is 90.35%. Comparing base (54170d8) to head (f351caa).

Files	Patch %	Lines
anesthetic/plot.py	1.53%	128 Missing ⚠️
anesthetic/plotting/_matplotlib/hist.py	62.50%	6 Missing ⚠️
anesthetic/plotting/_core.py	50.00%	2 Missing ⚠️

Additional details and impacted files

@@             Coverage Diff             @@
##            master     #353      +/-   ##
===========================================
- Coverage   100.00%   90.35%   -9.65%     
===========================================
  Files           36       36              
  Lines         3043     3195     +152     
===========================================
- Hits          3043     2887     -156     
- Misses           0      308     +308     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[AdamOrmondroyd]

Getting the docs to work properly for an optional dependency seems like a pain, so I've copied the fastkde approach of just putting a link to margarine

[htjb]

I don't understand, what was the issue here?

[AdamOrmondroyd]

There's probably a few layers to this, but part of it is that margarine only contains submodules (so from margarine.maf import MAF works but import margarine; margarine.maf.MAF does not, and I couldn't get to the bottom of how to refer to them properly.

You'll be able to see the error messages in the earlier checks

[AdamOrmondroyd]

(personally I'd change the margarine api but I'm just a disruptive influence)

[AdamOrmondroyd]

Even changing the api hasn't made it straightforward - @lukashergt is the king when it comes to the docs

[lukashergt]

To explain a bit more, the issue was that the docstring contained :class:`margarine.maf` , which tells autodoc to try and link to the documentation of a class called maf in a package called margarine, which autodoc wasn't able to find, though. Two issues here:

1. There is no dedicated documentation for the class :class:`margarine.maf` , there is only documentation for the method :meth:`margarine.maf.MAF` .
2. In order for autodoc to find the margarine documentation, you need to tell it where to look. In docs/source/conf.py you can find a dictionary that tells autodoc where to do that for matplotlib, pandas, and Co. You'll need to add the margarine readthedocs url to that dictionary in order to get the automatic linking to work.

[AdamOrmondroyd]

1. I think :class: is the right thing, since margarine.maf is a submodule, and MAF is a class inside that submodule.
2. I'd tried adding this and couldn't get it to work 😢

[lukashergt]

Ah, it appears that the margarine docs don't build all the needed reference targets. I've created a PR htjb/margarine#54 that uses more of the autodoc power that should hopefully remedy that.

[htjb]

Ahh okay I see! Thanks @lukashergt I will have a look at your PR on margarine asap.

[AdamOrmondroyd]

tests now failing as expected (I'd forgotten to add margarine to the 'all' dependency!)

[htjb]

@AdamOrmondroyd Cant remember what needs doing here? is it just the tests need tidying? and master merging?

[AdamOrmondroyd]

@AdamOrmondroyd Cant remember what needs doing here? is it just the tests need tidying? and master merging?

The tests need a rethink, because simply adding nde everywhere we previously saw kde slows everything down too much

Also at least one of them appears to have failed at the last time of running, and you're missing some coverage

[htjb]

Ahh yes the NDEs are not particularly quick... maybe like a few minutes for a few thousands of samples (dependent on the architecture). Has the potential to slow the tests down...

Looks like the test failed because of a version mismatch between TensorFlow and TensorFlow Probability. What does the extras flag turn on in the tests?

[lukashergt]

@AdamOrmondroyd Cant remember what needs doing here? is it just the tests need tidying? and master merging?

This also needed waiting for the margarine docs to work, which is now the case. So now the NDE docstrings properly link to the margarine docs.

[htjb]

@AdamOrmondroyd pointed out that I might have set the code up to train a new NDE for every panel in the corner plot if I just replaced KDE with NDE in the plotting script. I think it should be possible to train an nD NDE and use this to plot contours on a corner plot. Need to check over the code though.