Replace objax with flax #68
Conversation
|
Directory |
This folder should be deleted. For now we decided that no training scripts were going to be included in the release. |
| from scico.data import _objax_data_path | ||
| from scico.objax import DnCNN_Net | ||
| from scico.data import _flax_data_path | ||
| from scico.flax import DnCNNNet, load_weights | ||
| from scico.typing import JaxArray |
There was a problem hiding this comment.
Any reason for name change from DnCNN_Net to DnCNNNet? The former is a bit more readable.
There was a problem hiding this comment.
I am trying to follow the naming convention in the style guide. Underscores are not used in names of classes or names of exceptions.
There was a problem hiding this comment.
Ah yes, good point. The three 'N's in a row aren't great for readability, so perhaps we should consider amending the style guide for such cases, but let's leave it as it is for now. FWIW, this issue has been encountered before, and one of the suggested solutions is a PEP-8 exception to include underscores.
@Michael-T-McCann @lukepfister : Any thoughts on this?
scico/flax.py
Outdated
| dtype=self.dtype, | ||
| ) | ||
|
|
||
| # Processing |
There was a problem hiding this comment.
This comment is a bit mysterious. Also, see new comment style guidance in the style doc.
There was a problem hiding this comment.
Edited comment to add more detail and follow style guide.
scico/flax.py
Outdated
| strides=self.strides, | ||
| name="conv_end", | ||
| )(y) | ||
| return base - y # Residual-like network |
There was a problem hiding this comment.
According to new comment style, should be # residual-like network.
scico/functional/_denoiser.py
Outdated
| deviation: L (low) = 0.06, M (mid) = 0.1, H (high) = 0.2, where the standard | ||
| deviations are with respect to data in the range [0, 1]. | ||
| variant: Identify the DnCNN model to be used. | ||
| Options are '6L', '6M' (default), '6H', '17L', '17M', |
There was a problem hiding this comment.
Indentation looks wrong for an Args section, and the corresponding Parameters section in the docs doesn't render correctly.
There was a problem hiding this comment.
Changed it. It seems OK in the documentation generated. Let me know if it is still not how it is supposed to be rendered.
scico/test/test_flax.py
Outdated
|
|
||
|
|
||
| def test_DnCNN_train(testobj): | ||
| # Test training flag for training |
There was a problem hiding this comment.
Final "for training" intentional or typo?
There was a problem hiding this comment.
Intentional. A train flag argument is passed when applied the model. I am checking that the processing changes when the flag is True (i.e. set for training) or False (i.e. set for testing). I can remove this if deemed too confusing.
There was a problem hiding this comment.
Perhaps some rephrasing to minimize confusion, e.g. Test effect of training flag?
There was a problem hiding this comment.
Rephrased as suggested.
Replaced Objax utilities with Flax.