claritychallenge / clarity

Clarity Challenge toolkit - software for building Clarity Challenge systems
https://claritychallenge.github.io/clarity
MIT License
121 stars 51 forks source link

Cad1task1 baseline2 #300

Closed groadabike closed 1 year ago

groadabike commented 1 year ago

This PR for release v0.3.2

What's included?

In Cad 1 - Task 1:

In Cad1 - Task 2:

Other changes:

jonbarker68 commented 1 year ago

I guess there is a danger of complicating things here. We probably want to keep changes on 0.3.x as minimal as possible and constrained to the recipe code.

I would keep the changes to the recipe in 0.3.2 as small as possible, i.e. stick to adding the new demixing functionality. Then we'll refactor all the recipes to make better use of the existing write_signal code in the ongoing refactoring work that will be released in August for 0.4.0

(i.e. the write_signal code duplication is already in the 0.3. release so this small update is not the place to be fixing it)

Jon

On Thu, 20 Apr 2023 at 10:55, Gerardo Roa Dabike @.***> wrote:

@.**** commented on this pull request.

In recipes/cad1/task1/baseline/enhance.py https://github.com/claritychallenge/clarity/pull/300#discussion_r1172361182 :

     if n_clipped > 0:

logger.warning(f"Writing {filename}: {n_clipped} samples clipped")

  • np.clip(enhanced, -1.0, 1.0, out=enhanced)

@ns-rse https://github.com/ns-rse, I always confused about the cherry-pick. What would be better in this case, charry-pick or just add another new commit copying these function?

— Reply to this email directly, view it on GitHub https://github.com/claritychallenge/clarity/pull/300#discussion_r1172361182, or unsubscribe https://github.com/notifications/unsubscribe-auth/ACO7SJABAO4CKBGQ7NNYO4TXCEBZPANCNFSM6AAAAAAXELDSJY . You are receiving this because you commented.Message ID: @.***>

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

groadabike commented 1 year ago

@jonbarker68 , After closing v0.3.2, I will submit another PR updating main using the improvements of this version. I.e., adding the flac and 16 bit things

jonbarker68 commented 1 year ago

Great. That looks much clearer. There is now just a parameter missing in the call to decompose_signal in test_enhance_task1.py

groadabike commented 1 year ago

Thank you,

Yeah, I fixed that error in the test and added the new ones. Hope is ok for merging

On Thu, 11 May 2023 at 09:47, Jon Barker @.***> wrote:

Great. That looks much clearer. There is now just a parameter missing in the call to decompose_signal in test_enhance_task1.py

— Reply to this email directly, view it on GitHub https://github.com/claritychallenge/clarity/pull/300#issuecomment-1543592575, or unsubscribe https://github.com/notifications/unsubscribe-auth/AED2G2WERLIG5BXSWLN5T5DXFSRQXANCNFSM6AAAAAAXELDSJY . You are receiving this because you were assigned.Message ID: @.***>

-- Gerardo Roa Dabike Ingeniero Civil Informatico