Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix operand array error #79

Merged
merged 7 commits into from
May 10, 2023
Merged

Fix operand array error #79

merged 7 commits into from
May 10, 2023

Conversation

dboulware
Copy link
Contributor

This fixes a bug caused by passing in a read only array into the fft routine. The fix creates a new empty array for the output that matches the shape and datatype of the array passed in.

@dboulware dboulware requested a review from jhazentia May 9, 2023 16:47
jhazentia
jhazentia requested changes May 9, 2023
View reviewed changes
Copy link
Member

@jhazentia jhazentia left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks like this is failing tests

@aromanielloNTIA
Copy link
Member

The tests fail because in Doug's changes, if there is no FFT windowing, then the FFT is performed on an empty array. I suggest making a copy of the array instead, and only doing so when needed (i.e., in the case where windowing is applied and the input data is large enough that NumExpr will be used to do so). I can make this change and update the tests to cover the read-only input data case. I will also check to see if a similar problem pops up elsewhere where NumExpr is used.

@aromanielloNTIA
Copy link
Member

I did not see the same problem happening with NumExpr usage inside other signal processing functions. However, I modified signal processing unit tests so that all tests for functions which use NumExpr for large arrays will be tested with read-only array inputs. This should help catch any similar problem in the future. All tests are passing now.

@aromanielloNTIA aromanielloNTIA merged commit cc91045 into master May 10, 2023
@aromanielloNTIA aromanielloNTIA deleted the OperandArrayError branch May 10, 2023 19:51
aromanielloNTIA added a commit that referenced this pull request May 10, 2023
@aromanielloNTIA
Squashed commit of the following:
b926b4c 
commit cc91045
Merge: 36339d7 23bb18b
Author: Anthony Romaniello <66272872+aromanielloNTIA@users.noreply.github.com>
Date:   Wed May 10 13:51:43 2023 -0600

    Merge pull request #79 from NTIA/OperandArrayError

    Fix operand array error

commit 23bb18b
Author: Anthony Romaniello <aromaniello@ntia.gov>
Date:   Wed May 10 14:44:36 2023 -0400

    use correct IQ object inside loop

commit 97f400b
Author: Anthony Romaniello <aromaniello@ntia.gov>
Date:   Wed May 10 13:45:44 2023 -0400

    add readonly inputs to numexpr function tests

commit 9b101e6
Author: Anthony Romaniello <aromaniello@ntia.gov>
Date:   Wed May 10 13:26:41 2023 -0400

    Update FFT tests to handle large and immutable input data

commit 06c4383
Author: Anthony Romaniello <aromaniello@ntia.gov>
Date:   Wed May 10 13:25:50 2023 -0400

    change operand array error fix

commit d0fea34
Author: Doug Boulware <dboulware@ntia.doc.gov>
Date:   Tue May 9 10:30:42 2023 -0600

    set data type of empty array.

commit c36be9a
Author: Doug Boulware <dboulware@ntia.doc.gov>
Date:   Tue May 9 10:12:50 2023 -0600

    use array shape instead of length.

commit c8d25fb
Author: Doug Boulware <dboulware@ntia.doc.gov>
Date:   Tue May 9 09:54:11 2023 -0600

    Use writable variable in fft.
aromanielloNTIA added a commit that referenced this pull request May 10, 2023
@aromanielloNTIA
Squashed commit of the following:
0b36375 
commit cc91045
Merge: 36339d7 23bb18b
Author: Anthony Romaniello <66272872+aromanielloNTIA@users.noreply.github.com>
Date:   Wed May 10 13:51:43 2023 -0600

    Merge pull request #79 from NTIA/OperandArrayError

    Fix operand array error

commit 23bb18b
Author: Anthony Romaniello <aromaniello@ntia.gov>
Date:   Wed May 10 14:44:36 2023 -0400

    use correct IQ object inside loop

commit 97f400b
Author: Anthony Romaniello <aromaniello@ntia.gov>
Date:   Wed May 10 13:45:44 2023 -0400

    add readonly inputs to numexpr function tests

commit 9b101e6
Author: Anthony Romaniello <aromaniello@ntia.gov>
Date:   Wed May 10 13:26:41 2023 -0400

    Update FFT tests to handle large and immutable input data

commit 06c4383
Author: Anthony Romaniello <aromaniello@ntia.gov>
Date:   Wed May 10 13:25:50 2023 -0400

    change operand array error fix

commit d0fea34
Author: Doug Boulware <dboulware@ntia.doc.gov>
Date:   Tue May 9 10:30:42 2023 -0600

    set data type of empty array.

commit c36be9a
Author: Doug Boulware <dboulware@ntia.doc.gov>
Date:   Tue May 9 10:12:50 2023 -0600

    use array shape instead of length.

commit c8d25fb
Author: Doug Boulware <dboulware@ntia.doc.gov>
Date:   Tue May 9 09:54:11 2023 -0600

    Use writable variable in fft.
aromanielloNTIA added a commit that referenced this pull request May 11, 2023
@aromanielloNTIA
Squashed commit of the following:
dee7180 
commit cc91045
Merge: 36339d7 23bb18b
Author: Anthony Romaniello <66272872+aromanielloNTIA@users.noreply.github.com>
Date:   Wed May 10 13:51:43 2023 -0600

    Merge pull request #79 from NTIA/OperandArrayError

    Fix operand array error

commit 23bb18b
Author: Anthony Romaniello <aromaniello@ntia.gov>
Date:   Wed May 10 14:44:36 2023 -0400

    use correct IQ object inside loop

commit 97f400b
Author: Anthony Romaniello <aromaniello@ntia.gov>
Date:   Wed May 10 13:45:44 2023 -0400

    add readonly inputs to numexpr function tests

commit 9b101e6
Author: Anthony Romaniello <aromaniello@ntia.gov>
Date:   Wed May 10 13:26:41 2023 -0400

    Update FFT tests to handle large and immutable input data

commit 06c4383
Author: Anthony Romaniello <aromaniello@ntia.gov>
Date:   Wed May 10 13:25:50 2023 -0400

    change operand array error fix

commit d0fea34
Author: Doug Boulware <dboulware@ntia.doc.gov>
Date:   Tue May 9 10:30:42 2023 -0600

    set data type of empty array.

commit c36be9a
Author: Doug Boulware <dboulware@ntia.doc.gov>
Date:   Tue May 9 10:12:50 2023 -0600

    use array shape instead of length.

commit c8d25fb
Author: Doug Boulware <dboulware@ntia.doc.gov>
Date:   Tue May 9 09:54:11 2023 -0600

    Use writable variable in fft.

commit 36339d7
Merge: fab8bdd 0b07535
Author: Anthony Romaniello <66272872+aromanielloNTIA@users.noreply.github.com>
Date:   Mon May 8 14:38:58 2023 -0600

    Merge pull request #77 from NTIA/sea-action-garbage-collection

    Ray Actor to supervise IQ processing in SEA action; add DSP unit tests

commit 0b07535
Author: Anthony Romaniello <aromaniello@ntia.gov>
Date:   Mon May 8 11:18:06 2023 -0400

    complex array exception handling in quantile filter

commit d2e2db3
Author: Anthony Romaniello <aromaniello@ntia.gov>
Date:   Mon May 8 11:10:43 2023 -0400

    fix numexpr casting in fft windowing

commit 5caf4f7
Author: Anthony Romaniello <aromaniello@ntia.gov>
Date:   Mon May 8 10:45:56 2023 -0400

    ensure all intended tests run

commit 7f7c37c
Author: Anthony Romaniello <aromaniello@ntia.gov>
Date:   Mon May 8 10:39:43 2023 -0400

    fix elif -> else

commit 748709e
Author: Anthony Romaniello <aromaniello@ntia.gov>
Date:   Fri May 5 13:42:14 2023 -0400

    only use NumExpr for large arrays in APD calculations

commit e21cfa0
Author: Anthony Romaniello <aromaniello@ntia.gov>
Date:   Fri May 5 13:37:24 2023 -0400

    add remaining signal processing tests

commit 6148881
Author: Anthony Romaniello <aromaniello@ntia.gov>
Date:   Fri May 5 13:12:21 2023 -0400

    add array size threshold for NumExpr usage

commit ecf7d02
Author: Anthony Romaniello <aromaniello@ntia.gov>
Date:   Thu May 4 21:45:01 2023 -0400

    add unit tests

commit 65fb3be
Author: Anthony Romaniello <aromaniello@ntia.gov>
Date:   Thu May 4 21:43:12 2023 -0400

    fix typo

commit 584f5f9
Author: Anthony Romaniello <aromaniello@ntia.gov>
Date:   Thu May 4 16:44:39 2023 -0400

    fix incorrect comment

commit db1300f
Author: Anthony Romaniello <aromaniello@ntia.gov>
Date:   Wed May 3 17:15:35 2023 -0400

    ray@^2.4.0

commit 9fdc5c3
Author: Anthony Romaniello <aromaniello@ntia.gov>
Date:   Wed May 3 17:10:17 2023 -0400

    add docstrings and type hints

commit 6b4dbef
Author: Anthony Romaniello <aromaniello@ntia.gov>
Date:   Tue May 2 21:33:07 2023 -0400

    fix PVT result unpacking

commit 3edb3d7
Author: Anthony Romaniello <aromaniello@ntia.gov>
Date:   Tue May 2 21:15:47 2023 -0400

    fix actor instantiation

commit 97102fa
Author: Anthony Romaniello <aromaniello@ntia.gov>
Date:   Tue May 2 21:08:39 2023 -0400

    use a single parent actor, do not block in parent run

commit f7a7427
Author: Anthony Romaniello <aromaniello@ntia.gov>
Date:   Tue May 2 20:47:28 2023 -0400

    test supervisor actor for concurrency

commit 0873c5a
Author: Anthony Romaniello <aromaniello@ntia.gov>
Date:   Tue May 2 16:22:38 2023 -0400

    roll back ray version

commit c59a3d6
Author: Anthony Romaniello <aromaniello@ntia.gov>
Date:   Tue May 2 14:16:29 2023 -0400

    reduce debug clutter for expected behavior

commit 9b35c10
Author: Anthony Romaniello <aromaniello@ntia.gov>
Date:   Tue May 2 11:00:17 2023 -0400

    end of action garbage collect

commit 8224799
Author: Anthony Romaniello <aromaniello@ntia.gov>
Date:   Thu Apr 27 14:27:12 2023 -0400

    fix channel power summary retrieval

commit 518c2e8
Author: Anthony Romaniello <aromaniello@ntia.gov>
Date:   Thu Apr 27 14:14:04 2023 -0400

    fix num_returns for generate_data_product

commit 3b8f941
Author: Anthony Romaniello <aromaniello@ntia.gov>
Date:   Thu Apr 27 13:58:30 2023 -0400

    delay ray.get using generators

commit eb3de44
Author: Anthony Romaniello <aromaniello@ntia.gov>
Date:   Wed Apr 26 18:56:07 2023 -0400

    set Ray dashboard host address to 0.0.0.0

commit 4b7ae39
Author: Anthony Romaniello <aromaniello@ntia.gov>
Date:   Wed Apr 26 18:14:39 2023 -0400

    improve ray performance using generators

commit 0d68dd3
Author: Anthony Romaniello <aromaniello@ntia.gov>
Date:   Wed Apr 26 18:13:47 2023 -0400

    speed up computations when downsampling APD

commit 4c2ab0a
Author: Anthony Romaniello <aromaniello@ntia.gov>
Date:   Wed Apr 26 13:28:22 2023 -0400

    debugging

commit 585d5b1
Author: Anthony Romaniello <aromaniello@ntia.gov>
Date:   Wed Apr 26 12:15:19 2023 -0400

    ray debug

commit 9faf084
Author: Anthony Romaniello <aromaniello@ntia.gov>
Date:   Tue Apr 25 20:14:37 2023 -0400

    remove unused import

commit 665b496
Author: Anthony Romaniello <aromaniello@ntia.gov>
Date:   Tue Apr 25 20:03:31 2023 -0400

    memtest no manual GC

commit af36126
Author: Anthony Romaniello <aromaniello@ntia.gov>
Date:   Tue Apr 25 19:18:36 2023 -0400

    bump patch version

commit adc8a76
Author: Anthony Romaniello <aromaniello@ntia.gov>
Date:   Tue Apr 25 19:16:17 2023 -0400

    disable Ray debugging tools

commit 31b3acc
Author: Anthony Romaniello <aromaniello@ntia.gov>
Date:   Tue Apr 25 19:07:07 2023 -0400

    avoid all logging in DSP functions

    DSP methods have been tested and documented, and avoiding use of loggers within them can help streamline parallelized action implementations

commit 8bfea89
Author: Anthony Romaniello <aromaniello@ntia.gov>
Date:   Tue Apr 25 18:48:03 2023 -0400

    remove redundant dictionary comprehension

commit 9143c45
Author: Anthony Romaniello <aromaniello@ntia.gov>
Date:   Tue Apr 25 17:59:38 2023 -0400

    add ntia-core classification to metadata

commit 657d5f8
Author: Anthony Romaniello <aromaniello@ntia.gov>
Date:   Tue Apr 25 17:41:20 2023 -0400

    Ray debugging

commit 2001dc9
Author: Anthony Romaniello <aromaniello@ntia.gov>
Date:   Tue Apr 25 16:32:31 2023 -0400

    bump ray to >=2.4.0

commit 7d560f9
Author: Anthony Romaniello <aromaniello@ntia.gov>
Date:   Tue Apr 25 16:32:11 2023 -0400

    fix Ray function call

commit 28a346a
Author: Anthony Romaniello <aromaniello@ntia.gov>
Date:   Tue Apr 25 16:31:19 2023 -0400

    explicitly initialize Ray

commit 9c9d72e
Author: Anthony Romaniello <aromaniello@ntia.gov>
Date:   Tue Apr 25 16:30:05 2023 -0400

    reduce already-parallel FFT process to 1 worker

commit dee3c5d
Author: Anthony Romaniello <aromaniello@ntia.gov>
Date:   Tue Apr 25 16:29:26 2023 -0400

    remove log statements in methods run by Ray workers

commit 5e6b3c7
Merge: 06df778 fab8bdd
Author: Anthony Romaniello <aromaniello@ntia.gov>
Date:   Mon Apr 24 17:59:08 2023 -0400

    Merge branch 'master' into sea-action-garbage-collection

commit 06df778
Author: Anthony Romaniello <aromaniello@ntia.gov>
Date:   Mon Apr 24 14:43:50 2023 -0400

    garbage collection
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@aromanielloNTIA aromanielloNTIA aromanielloNTIA left review comments

@jhazentia jhazentia jhazentia approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@dboulware @aromanielloNTIA @jhazentia