move creation of linregress.json to test step

[mshannon-sil]

This PR moves the creation of linregress.json to the test step, as discussed in issue #1114. This way, it's only created once rather than each time quality estimation is run, and the estimate_quality method takes in a path to the linregress.json file rather than a path to the verse test scores.

---

This change is 

[benjaminking]

Good work identifying this change. It seems like an improvement, not only for the user, but also for the architecture -- it makes the SILNLP a lot cleaner.

@benjaminking reviewed 5 files and all commit messages, and made 2 comments.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on mshannon-sil).

---

silnlp/nmt/experiment.py line 138 at r1 (raw file):

                    self.produce_multiple_translations,
                    self.save_confidences,
                    bool(quality_estimation),

While we're doing this, are we able to combine the quality estimation and the linear regression parameters? For example, can we only pass the linear regression path when we want to perform quality estimation? And then only perform quality estimation in translate when a valid linear regression path is passed in?

[mshannon-sil]

silnlp/nmt/experiment.py line 138 at r1 (raw file):

Previously, benjaminking (Ben King) wrote…

While we're doing this, are we able to combine the quality estimation and the linear regression parameters? For example, can we only pass the linear regression path when we want to perform quality estimation? And then only perform quality estimation in translate when a valid linear regression path is passed in?

Yeah, I think they should be combinable. Do you think we should keep them as separate flags in the command so that the user can choose to not specify the linear regression path if they don't have to (i.e., the linregress.json is located in the same experiment folder)? And just later in the code use the linregress.json file not being None as the flag for quality estimation?

[benjaminking]

@benjaminking made 1 comment.
Reviewable status: 2 of 5 files reviewed, 1 unresolved discussion (waiting on mshannon-sil).

---

silnlp/nmt/experiment.py line 138 at r1 (raw file):

Previously, mshannon-sil wrote…

Yeah, I think they should be combinable. Do you think we should keep them as separate flags in the command so that the user can choose to not specify the linear regression path if they don't have to (i.e., the linregress.json is located in the same experiment folder)? And just later in the code use the linregress.json file not being None as the flag for quality estimation?

Yes, that's probably fine to keep them as separate options so that a power user has the option to use different linear regression parameters. We'll probably want the description of the linregress parameter to make it clear that it should not be included in standard usage.

[mshannon-sil]

@mshannon-sil made 1 comment.
Reviewable status: 1 of 5 files reviewed, 1 unresolved discussion (waiting on benjaminking).

---

silnlp/nmt/experiment.py line 138 at r1 (raw file):

Previously, benjaminking (Ben King) wrote…

Yes, that's probably fine to keep them as separate options so that a power user has the option to use different linear regression parameters. We'll probably want the description of the linregress parameter to make it clear that it should not be included in standard usage.

Done.

[benjaminking]

@benjaminking reviewed 4 files and all commit messages, made 1 comment, and resolved 1 discussion.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on mshannon-sil).