Fix double macro argument reference causing double yield in [SD]Float#map

[mike-bourgeous]

Purpose

This pull request proposes a fix for #10, where Numo::SFloat#map and Numo::DFloat#map would yield each value to the block twice.

Summary of Changes

• Replace the m_num_to_data macro in float_macro.h with an inline function
• Add a test for the number of times map calls its block

Testing

• repro.rb prints FAIL without the change and now prints PASS
• rake test fails without the change and passes with the change
• mb-sound tests pass with the change
  
  
• Manual testing

  bundle exec ruby -rnumo/narray/alt -e'i = 0; Numo::SFloat.zeros(5).map { i += 1 } ; puts i'
  # prints 10 before the change
  # prints 5 after the change

Related Issues

Closes #10

[mike-bourgeous]

repro.rb

#!/usr/bin/env ruby
require 'bundler/setup'
require 'numo/narray/alt'

if ARGV.include?('--debug')
  # Shorter test with debugging output
  Numo::NArray.debug = true
  classes = [Numo::SFloat, Numo::DFloat, Numo::SComplex]
else
  # Full test
  names = Numo.constants - [:NMath, :Struct, :RObject, :NArray, :Bit]
  classes = names.sort.map { |n| Numo.const_get(n) }.uniq
end

classes.each do |klass|
  puts "\n\n\e[1m#{klass}:\e[0m"
  count = 0
  result = klass.zeros(5).map { |_| STDOUT.write(" #{count}") ; count += 1 }
  puts "\n  Block called #{count} times: #{result.inspect.sub("\n", ": ")} -- #{count == 5 ? "\e[32mPASS\e[0m" : "\e[31mFAIL\e[0m"}"
end

Before

After

[mike-bourgeous]

I saw that the commitlint workflow failed, so I reworded messages without squashing any commits. If you prefer, I can merge all of the commits into two -- one to create the failing test, and one to add the fix.

[yoshoku]

This gem usually uses the f_ prefix for inline function naming. It would be great if you could change this to f_num_to_data.

[yoshoku]

@mike-bourgeous Thank you for finding and fixing this bug. I was looking into why it does not occur with numo-narray v0.9.2.1. The change to the m_num_to_data macro came from commit 44f3a86, which had not been released yet: ruby-numo/numo-narray@44f4a86. I think this change is intended to replicate numpy's behavior:

>>> import numpy as np
>>> np.array([1, None, 1], dtype=np.float32)
array([ 1., nan,  1.], dtype=float32)

The commitlint workflow is failing again, but I believe it will pass if you make the entire commit message lowercase.
I also left a comment about the inline function name, so I would appreciate it if you could address that as well.

[mike-bourgeous]

@yoshoku In addition to your observations, I think the way the m_map macro is used might be another key change that allowed the double reference to have side effects. [never mind; I now see how the templating worked in the original numo-narray]

I have applied your suggestions and will be sure to set up your git hooks if I make any future contributions. Let me know if anything else is required for this PR.

[yoshoku]

Thank you for your contribution 👍

[yoshoku]

@mike-bourgeous I have released this fix as v0.10.5. Thanks so much for your help 🤝
https://rubygems.org/gems/numo-narray-alt/versions/0.10.5

[mike-bourgeous]

@yoshoku Awesome, thank you! Happy to help.