Rename classifier (for NRE) to critic

[bkmi]

What does this implement/fix? Explain your changes

The name classifier is inaccurate. Rather, a general name such as critic is more appropriate. The critic can be transformed into a classifier using the sigmoid (or other transformation). It can be made to estimate the likelihood-to-evidence ratio by simple evaluation. However, I think keeping the name of the network distinct from the RatioEstimator class is a good idea.

Does this close any currently open issues?

#1079

Any relevant code examples, logs, error output, etc?

Nope

Any other comments?

This cannot be applied until #1097 has been pulled!

Checklist

Put an x in the boxes that apply. You can also fill these out after creating the PR. If you're unsure about any of them, don't hesitate to ask. We're here to help! This is simply a reminder of what we are going to look for before merging your code.

• [x] I have read and understood the contribution guidelines
• [x] I agree with re-licensing my contribution from AGPLv3 to Apache-2.0.
• [x] I have commented my code, particularly in hard-to-understand areas
• [x] I have added tests that prove my fix is effective or that my feature works
• [x] I have reported how long the new tests run and potentially marked them with pytest.mark.slow.
• [x] New and existing unit tests pass locally with my changes
• [x] I performed linting and formatting as described in the contribution guidelines
• [x] I rebased on main (or there are no conflicts with main)

[bkmi]

I will follow up after Friday.

[bkmi]

@janfb, I used critic since it was shorter than discriminator. Also critic is the name used in WGAN where the "discriminator" network does not act as a binary classifier. We are in that situation, so I chose the name critic. However, I don't have a strong opinion except not to call it a classifier. There's a slight bias against discriminator for me since discriminators in GANs are classifiers.

let me know how you like it and I'll change to to that so we can get this merged.

[michaeldeistler]

I would also prefer to not change this right now -- users will already get a bunch of warnings when the new version is released (thinning for MCMC, importing posterior_nn) so I would prefer to not change this.

[bkmi]

okay, I'll follow your lead, but I would argue that maybe having one big change with many warnings will be simpler than adapting code once, then again for a later additional change.

[janfb]

Hi @bkmi to move on with this PR I suggest the following:

As we would prefer to not do the renaming to critic, we should drop those changes. The PR also contains a couple of other changes that we do want to keep though. However, it seems that those changes are contained in your other PR as (see #1097 ). Thus, could you please close this PR and double check that all not renaming related changes end up in #1097 ?

Thanks a lot! 🙏