recipes for ICASSP #120
recipes for ICASSP #120
Conversation
Codecov Report
@@ Coverage Diff @@
## main #120 +/- ##
==========================================
- Coverage 85.19% 85.19% -0.01%
==========================================
Files 32 37 +5
Lines 3782 3802 +20
==========================================
+ Hits 3222 3239 +17
- Misses 560 563 +3
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. |
There was a problem hiding this comment.
The README.md (or a copy of the text) could be placed as a document under docs/icassp2023.md and then in docs/index.rst it could be referenced with...
Welcome to pyClarity's documentation!
=====================================
.. toctree::
:maxdepth: 1
:caption: Getting Started
introduction
installation
usage
icassp2023
with the last line being the addition, any name can be used for the Markdown just need to match the relative path and filename in the index.rst and omit the .md.
These pages are then built automatically by a GitHub action (which I'm about to revise to only run on tagged releases) and are visible at https://claritychallenge.org/clarity/
| @@ -0,0 +1,65 @@ | |||
| import json | |||
| import logging | |||
| import os | |||
There was a problem hiding this comment.
Perhaps just preference based on experience but I find pathlib to be more intuitive and readable and generally easier to use than os.
| import os | |
| import pathlib |
There was a problem hiding this comment.
I also prefer pathlib over os.
Is an object-oriented library for working with paths.
Pathlib has nice things like mkdir that make the parents and validate if exist
Path('test_dir').mkdir(exist_ok=True, parents=True)
|
|
||
| @hydra.main(config_path=".", config_name="config") | ||
| def enhance(cfg: DictConfig) -> None: | ||
| enhanced_folder = os.path.join(cfg.path.exp_folder, "enhanced_signals") |
There was a problem hiding this comment.
With pathlib this would I think become.
enhanced_folder = Path(cfg.path.exp_folder) / "enhanced_signals"
enchanced_folder.mkdir(parents=True, exist_ok=True)
The parents=True is of course optional but it will then create any parent directories defined in cfg.path.exp_folder that don't exist.
| logger.warning(f"Writing {filename}: {n_clipped} samples clipped") | ||
| np.clip(enhanced, -1.0, 1.0, out=enhanced) | ||
| signal_16 = (32768.0 * enhanced).astype(np.int16) | ||
| wavfile.write(os.path.join(enhanced_folder, filename), fs, signal_16) |
There was a problem hiding this comment.
If switching to using pathlib this would become...
| wavfile.write(os.path.join(enhanced_folder, filename), fs, signal_16) | |
| wavfile.write(enhanced_folder / filename, fs, signal_16) |
There was a problem hiding this comment.
Yeah, a much cleaner code
| import hashlib | ||
| import json | ||
| import logging | ||
| import os |
There was a problem hiding this comment.
As with enhance.py the pathlib module could be used.
| score_dict = read_csv_scores(unproc_si_file) | ||
| ave_score = np.mean(list(score_dict.values())) | ||
| logger.info( | ||
| "si_unproc.csv exists, and the average HASPI scores for unprocessed scenes is {:.4f}".format( |
There was a problem hiding this comment.
This could be an f-string as in other logging statements, would take the form...
| "si_unproc.csv exists, and the average HASPI scores for unprocessed scenes is {:.4f}".format( | |
| f"si_unproc.csv exists, and the average HASPI scores for unprocessed scenes is {mean_score:.4f}" |
(accounting for above suggestion(s) on variable name.
There was a problem hiding this comment.
f-strings are more clear than .format(), especially when you have several variables in the same string.
| for line in csv_lines: | ||
| csv_writer.writerow(line) | ||
| score_dict = read_csv_scores(si_file) | ||
| ave_score = np.mean(list(score_dict.values())) |
There was a problem hiding this comment.
There are three types of averages (mean, median and mode) as this is the mean we can be explicit...
| ave_score = np.mean(list(score_dict.values())) | |
| mean_score = np.mean(list(score_dict.values())) |
(Sorry I used to be a statistician).
| for line in unproc_csv_lines: | ||
| csv_writer.writerow(line) | ||
| score_dict = read_csv_scores(unproc_si_file) | ||
| ave_score = np.mean(list(score_dict.values())) |
There was a problem hiding this comment.
As above could use mean_score to be explicit about what the value is.
| score_dict = read_csv_scores(si_file) | ||
| ave_score = np.mean(list(score_dict.values())) | ||
| logger.info( | ||
| "si.csv exists, and the average combined score is {:.4f}".format(ave_score) |
There was a problem hiding this comment.
Can be an f-string...
| "si.csv exists, and the average combined score is {:.4f}".format(ave_score) | |
| f"si.csv exists, and the average combined score is {mean_score:.4f}" |
...takes into account suggestion about mean.
ns-rse
assigned jonbarker68, groadabike, ns-rse and TuZehai and unassigned jonbarker68, groadabike, ns-rse and TuZehai
|
@ns-rse re the README.md: the README.md files have a special purpose in GitHub, i.e. they are displayed as you browse the source code. I think this is particularly important for the 'recipes' because i) we can then provide links to the GitHub recipe directories as landing pages for the challenges and ii) people can very easy see what is what as they navigate through the recipes sources on github. Note, the other recipes, i.e. CEC1, CPC1, CEC2 have these README's and it is similar to other toolkits, e.g. asteroid, where the recipes appear in the egs/ dir, https://github.com/asteroid-team/asteroid/tree/master/egs Is it possible to get the best of both by having the documentation build from the README.md files rather than from docs/ subdirectories? |
|
With reference to many of these comments it's probably worth noting that this is 99% duplicated from the cec2 recipes which weren't authored by me. As such most of these comments will apply to that code too.
When addressing these I'll apply those changes to the other as to make sure there is consistency in the repository.
Will.
________________________________
From: Gerardo Roa Dabike ***@***.***>
Sent: Tuesday, November 22, 2022 5:50:34 PM
To: claritychallenge/clarity ***@***.***>
Cc: jwillbailey ***@***.***>; Author ***@***.***>
Subject: Re: [claritychallenge/clarity] recipes for ICASSP (PR #120)
@groadabike commented on this pull request.
________________________________
In recipes/icassp_2023/baseline/evaluate.py<#120 (comment)>:
+ ave_score = np.mean(list(score_dict.values()))
+ logger.info(
+ "si.csv exists, and the average combined score is {:.4f}".format(ave_score)
+ )
+
+ if cfg.evaluate.cal_unprocessed_si:
+ with open(unproc_si_file, "w") as csv_f:
+ csv_writer = csv.writer(
+ csv_f, delimiter=",", quotechar='"', quoting=csv.QUOTE_MINIMAL
+ )
+ for line in unproc_csv_lines:
+ csv_writer.writerow(line)
+ score_dict = read_csv_scores(unproc_si_file)
+ ave_score = np.mean(list(score_dict.values()))
+ logger.info(
+ "si_unproc.csv exists, and the average HASPI scores for unprocessed scenes is {:.4f}".format(
f-strings are more clear than .format(), especially when you have several variables in the same string.
—
Reply to this email directly, view it on GitHub<#120 (comment)>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/AHDQ7YGDLNY3CWRE6P2EZ53WJUBWVANCNFSM6AAAAAASHVSUYU>.
You are receiving this because you authored the thread.Message ID: ***@***.***>
|
|
@jwillbailey - please just fix the issues in the ICASSP recipe and commit this. We can then raise a separate issue if we want to go back and change old recipes and make them consistent. I'm not saying that isn't worth doing, but it's not good if the PR for the ICASSP recipe touches all recipes. This will cause confusion later. The PRs should be linked to well-defined issues and be small where possible. Thx! |
|
@jonbarker68 I'll look into including the @jwillbailey I agree with @jonbarker68 about keeping the PR focused. Switching other recipes to use |
For ICASSP2023 the hearing aid amplification needs to be done as part of evaluate.py. Also no need to have a switch for evaluating the unprocessed signals as this is what the baseline enhance.py does in anycase. This code still needs tidying up.
|
@TuZehai - could you have a quick look at the recipes/icassp_2023 please? We need to release it Tuesday for the ICASSP challenge. The evaluate.py now has a fixed hearing aid amplifier and HASPI/HASQI scorer. It seems to be producing sensible scores but it would be good if you could check that it looks correct. The HA amplifier should be the same as we used as the baseline in CEC2. The enhance.py is just dummy code that takes the front mic pair and is the bit that entrants will need to replace. I've tried to make it reasonably clear how they would hook in. I've also defined a standard smaller dev set for more convenient rapid scoring; plus I've separated out the score reporting. Please check that the README looks clear. Thanks! |
|
Hi @jonbarker68, I just had a quick look at this and it looks nice and clear except for two issues:
|
|
Thanks. Good point re the haspi score. This was actually based on the
smaller set, but still 500 scenes of the 7500 evenly sampled so I’d be
surprised if very different. But it’s also using floating point rather than
CEC2 which used 16bit - but I’m still doing the tanh soft clip so I don’t
see why that would make a difference.
Think I’ll need to check more carefully. Perhaps the reference
normalisation is different or something.
On Mon, 28 Nov 2022 at 22:14, Zehai Tu ***@***.***> wrote:
Hi @jonbarker68 <https://github.com/jonbarker68>, I just had a quick look
at this and it looks nice and clear except for two issues:
- I suppose the development set of ICASSP should be the same as CEC2?
However, the baseline (i.e. NALR) HASPI is 0.332 according to the README,
while the CEC2 is 0.249.
- In the README file, it might be better to use python xxx.py instead
of python3 xxx.py to be consistent..
—
Reply to this email directly, view it on GitHub
<#120 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/ACO7SJEZ6LLEA5TMTCJTHUDWKUVEDANCNFSM6AAAAAASHVSUYU>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
--
Professor Jon Barker,
Department of Computer Science,
University of Sheffield
+44 (0) 114 222 1824
|
(1) normalise reference signals in evaluate.py (2) adjust Level1 for hasqi: if Level1=65dB, many signals will be "too quiet" after the processing of hearing loss ear, and the hasqi score will be just 0; thus adjust Level1 to 100dB (3) improve two functions in eb.py just in case some signals can be "too quiet" that no correlation is computed and return error (should return 0 after fixing)
Make eq=0 as default setting and move it to the last position so that hasqi_v2_be can correctly pass level to hasqi_v2
recipes for ICASSP