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.

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

Use ndarray.size instead of numpy.prod #180

Merged
merged 1 commit into from
Jun 24, 2022
Merged

Conversation

zariiii9003
Copy link
Contributor

The numpy.prod() call seems quite expensive:
image

The _calculate_num_samps_per_chan() call could be skipped entirely if the user requested to read all available data (=-1), right?

@zhindes
Copy link
Collaborator

zhindes commented Jun 24, 2022

The _calculate_num_samps_per_chan() call could be skipped entirely if the user requested to read all available data (=-1), right?

You mean in the StreamReaders? If so, no, _calculate_num_samps_per_chan converts the -1 to a real value.

Copy link
Collaborator

@zhindes zhindes left a comment

Choose a reason for hiding this comment

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

Good fix, thanks!

@zhindes zhindes merged commit 1f55863 into ni:master Jun 24, 2022
@zariiii9003
Copy link
Contributor Author

zariiii9003 commented Jun 24, 2022

You mean in the StreamReaders? If so, no, _calculate_num_samps_per_chan converts the -1 to a real value.

_read_analog_f_64() calls DAQmxReadAnalogF64 which accepts -1 to read all values. So unless _verify_array_shape is True, that seems unnecessary.
You could save ~30% time by checking _verify_array_shape first:

        if self._verify_array_shape:
            number_of_samples_per_channel = (
                self._task._calculate_num_samps_per_chan(
                    number_of_samples_per_channel))
    
            self._verify_array(data, number_of_samples_per_channel, True, True)

        return _read_analog_f_64(
            self._handle, data, number_of_samples_per_channel,
            timeout)

Edit: Seems like DAQmxReadAnalogF64 and numSampsPerChan=-1 is blocking when the task is not continuous. I guess that's undesirable.

@zariiii9003 zariiii9003 deleted the avoid_np_prod branch June 24, 2022 17:15
@zhindes
Copy link
Collaborator

zhindes commented Jun 24, 2022

Oh, right, great point. I was going to recommend that we move _calculate_num_samps_per_chan into _verify_array, but that wouldn't be quite right unless we made number_of_samples_per_channel and out-param. We want to use a consistent snapshot of the number_of_samples_per_channel to read for both _verify_array and _read_analog_f_64.

Our stream tests don't seem to cover anything useful for -1, so I'd want to include some new tests, some pseudocode:

for verify_array_shape in [True, False]:
   for acq_mode in [Finite, Continuous, On Demand]:
      # setup task
      # if continuous, wait for 0.1s
      if verify_array_shape:
         # allocate not enough data
         expect throw:
            read(-1)

      # allocate plenty of data
      read(-1)
      # validate we got as much data as we wanted (all samples, ~0.1s worth, or 1 sample depending on acq_mode)
      # validate good data

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants