Added regression test for clarity/predictor

[TuZehai]

Coverage is currently 67%

Test torch_stoi (extended version included) using randomly generated x and torch_msbg processed x; test for torchloudnorm (might not be used anywhere, but just in case)

[jwillbailey]

I'm getting a test failure
`regression test output differences for tests/regression/test_predictors.py::test_torch_msbg_stoi:

--- current
+++ tobe
@@ -1,2 +1,2 @@
-Torch MSBG STOILoss -0.46197593, ESTOILoss -0.32999617
+Torch MSBG STOILoss -0.46197575, ESTOILoss -0.32999593
`

when i run the test suite. Maybe consider rounding to 5dp? are these differences significant?

[TuZehai]

I kind of intended to set it to 8, because in torch_stoi lines 124 and 125, I added an EPS, which is different from the PyPi installed torch_stoi. I verified in my local machine and the output is consistent. Could you try to create a new conda environment to only install by pip install -e '.[tests,dev]', and have another go?

[jwillbailey]

I'm using a fresh environment started this morning. Have you altered the requirements in this branch?

[TuZehai]

I am using a new environment as well. Are you using a CUDA machine? Because my local machine has a GPU and the tensors will be in CUDA, and I have to transfer it back to CPU. If that is the cause, probably it is the best to round to 5

[TuZehai]

By the way, do you get constant output? I.e. Torch MSBG STOILoss -0.46197575, ESTOILoss -0.32999593

[jwillbailey]

I’m just running the script from within the suite but I do have an nVidia card and CUDA installed. I’ve just set up the environment using the install procedure so I don’t know without checking if it’s running with device=”cuda” Perhaps that is a cause? Will From: Zehai ***@***.***> Sent: 27 July 2022 14:25 To: ***@***.***> Cc: ***@***.***>; State ***@***.***> Subject: Re: [claritychallenge/clarity] Added regression test for clarity/predictor (PR #50) I am using a new environment as well. Are you using a CUDA machine? Because my local machine has a GPU and the tensors will be in CUDA, and I have to transfer it back to CPU. If that is the cause, probably it is the best to round to 5 — Reply to this email directly, view it on GitHub<#50 (comment)>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/AHDQ7YD4WZHSRYTWCJCZ5S3VWE2DJANCNFSM54ZE766Q>. You are receiving this because you modified the open/close state.Message ID: ***@***.***>

[jonbarker68]

Hi, Note, these tests will need to run and pass on GitHub using github actions -- it's kind of less important what they do on your local machine. You can test a github actions manually (i.e. before merging) by selecting the 'Actions' tab and then clicking on the 'Clarity Tests' action in the menu on the left, and then the 'Run Workflow' button and then selecting your branch from the workflow. This is another good reason for working in a subbranch. You can then make sure tests are passing in it before coming to put in a pull request. Note, our github actions test script currently runs the tests on linux machines and a suite of different python versions. Tune the precision of new tests to pass the github action, but please don't decrease the precision of existing tests. If tests are failing on your local machine then you might want to look at this framework for running GitHub actions locally via Docker https://github.com/nektos/act. But basically if they are failing locally but passing on GitHub then all is OK. Note, they regression tests are not like tests of logic. They are just meant to be picking up on unexpected changes (...I t think we'll eventually separate them out from the regular tests. i.e. we might actually be best to be running them in one environment but with a higher precision) Jon

…

On Wed, 27 Jul 2022 at 14:30, jwillbailey ***@***.***> wrote: I’m just running the script from within the suite but I do have an nVidia card and CUDA installed. I’ve just set up the environment using the install procedure so I don’t know without checking if it’s running with device=”cuda” Perhaps that is a cause? Will From: Zehai ***@***.***> Sent: 27 July 2022 14:25 To: ***@***.***> Cc: ***@***.***>; State ***@***.***> Subject: Re: [claritychallenge/clarity] Added regression test for clarity/predictor (PR #50) I am using a new environment as well. Are you using a CUDA machine? Because my local machine has a GPU and the tensors will be in CUDA, and I have to transfer it back to CPU. If that is the cause, probably it is the best to round to 5 — Reply to this email directly, view it on GitHub< #50 (comment)>, or unsubscribe< https://github.com/notifications/unsubscribe-auth/AHDQ7YD4WZHSRYTWCJCZ5S3VWE2DJANCNFSM54ZE766Q >. You are receiving this because you modified the open/close state.Message ID: ***@***.***> — Reply to this email directly, view it on GitHub <#50 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ACO7SJHN7RSCCX2FGEQNI7TVWE2WFANCNFSM54ZE766Q> . You are receiving this because you are subscribed to this thread.Message ID: ***@***.***>

-- Professor Jon Barker, Department of Computer Science, University of Sheffield +44 (0) 114 222 1824