Menu

#68 Mesh2HRTF python_port testing

3.0
closed
nobody
None
2022-02-07
2022-01-17
Sergejs
No

Hello,

I tried out the python_port and ran into 2 bugs which make me wonder if I am missing some prerequisites with Python 3.9.7 or the code needs more work. Basically, I can re-write the code to make it work, but I need to know that it is an actual bug

in https://sourceforge.net/p/mesh2hrtf/git/ci/feature/python_port/~/tree/mesh2hrtf/Output2HRTF/Python/Output2HRTF_Main.py

First I got an error at line 244, but that was an easy fix by commenting out 4 unnecessary lines of code:

# tmpPressure:
tmpPressure.append(tmpData)
frequencies.append(tmpFrequencies)
# else:
#     tmpPressure = tmpData
#     frequencies = tmpFrequencies~~~

But then on line 255 I get the more serious issue that I would like to confirm before I fix it:

Traceback (most recent call last):

  File "c:\hrtf_numcalc\2021_09 s_head_62k_24khz_full\output2hrtf.py", line 42, in <module>
    m2h.Output2HRTF_Main(projectPath, Mesh2HRTF_version, cpusAndCores,

  File "c:\hrtf_numcalc\mesh2hrtf-feature-python_port_211212\mesh2hrtf\Output2HRTF\Python\Output2HRTF_Main.py", line 122, in Output2HRTF_Main
    pressure, frequencies = read_pressure(ears, cpusAndCores,

  File "c:\hrtf_numcalc\mesh2hrtf-feature-python_port_211212\mesh2hrtf\Output2HRTF\Python\Output2HRTF_Main.py", line 255, in read_pressure
    pressure.append(tmpPressure[idx, :])

TypeError: list indices must be integers or slices, not tuple

As far as I know, Python does not allow indexing like this.

Best Regards,
Sergejs

Discussion

  • Sergejs

    Sergejs - 2022-01-17
    • Description has changed:

    Diff:

    --- old
    +++ new
    @@ -6,12 +6,12 @@
    
     First in  I got an error at line 244, but that was an easy fix by commenting out 4 unnecessary lines of code:
    
    -# if len(tmpPressure)&gt;0:
    + # if len(tmpPressure)&gt;0:
      tmpPressure.append(tmpData)
      frequencies.append(tmpFrequencies)
    - # else:
    - #     tmpPressure = tmpData
    - #     frequencies = tmpFrequencies~~~
    +  # else:
    +  #     tmpPressure = tmpData
    +  #     frequencies = tmpFrequencies~~~
    
    
     But then on line 255 I get the more serious issue that I would like to confirm before I fix it:
    
     
  • Sergejs

    Sergejs - 2022-01-17
    • Description has changed:

    Diff:

    --- old
    +++ new
    @@ -4,9 +4,9 @@
    
     in https://sourceforge.net/p/mesh2hrtf/git/ci/feature/python_port/~/tree/mesh2hrtf/Output2HRTF/Python/Output2HRTF_Main.py
    
    -First in  I got an error at line 244, but that was an easy fix by commenting out 4 unnecessary lines of code:
    +First  I got an error at line 244, but that was an easy fix by commenting out 4 unnecessary lines of code:
    
    - # if len(tmpPressure)&gt;0:
    + # tmpPressure:
      tmpPressure.append(tmpData)
      frequencies.append(tmpFrequencies)
       # else:
    
     
  • Fabian Brinkmann

    We are currently implementing testing for the code. It worked with a few minimal examples that we tested so far but there might be undiscovered bugs. Can you upload you project or is it too large?

     
  • Sergejs

    Sergejs - 2022-01-17

    OK, thanks, I will make a workaround for myself then.

    I am quite surprised that lines 244 and 255 did not cause any trouble in testing while it results in a crash for me. This code is always executed no mater which project, so I think it does not matter which data we use.

    Lets leave the question for now.

     
  • Fabian Brinkmann

    I think line 257 should be tmpPressure = [] not del tmpPressure. The way of indexing works with numpy arrays...

     
  • Fabian Brinkmann

    Ok, we checked the problem. A first test suggests that it is now fixed. Note that the latest work is on simplify_export.. branch and that this is work in progress. This branch creates data directiores source# instead of CPU#_Core', which makes the code simpler

     
  • Sergejs

    Sergejs - 2022-01-18

    Sound great!
    When I look at the changes they are very similar to the workaround I made last night - eliminating the sorting code which is most likely never used. Only in my case I then got an error on the next line: pressure = numpy.transpose(numpy.array(pressure), (2, 0, 1))
    I get: *** ValueError: axes don't match array
    it may be due to having only one frequency step per data file, but I need to look into it later.

    The interesting part:
    is the simplify_export_cpu_and_cores branch ready to run? I would like to simulate Kemar dummy 3D data and if the branch is ready, I could try it. Then I would report on the post processing using the latest Python code from that branch.

     
  • Fabian Brinkmann

    Let's say ready to test. All API changes that are planned vor Mesh2HRTF 1. 0 are there, but not everything is tested yet. We are usually working on it one to two days a week. I would say it's an early beta version.

    And the Matlab part in that branch is currently not working. There is a different person working on that.

     

    Last edit: Fabian Brinkmann 2022-01-18
  • Sergejs

    Sergejs - 2022-01-18

    I will test it this evening. If API is there, I can start re-coding the NumPyManager to support the new project structure. We will need to run a few simulations in the next weeks for the MSc thesis, and so far I get some error in all versions, but we will fix it.

     
  • Sergejs

    Sergejs - 2022-01-18

    So I tested the all_new simplify_export_cpu_and_cores branch. It works quite well with Point Source. I recompiled NumCalc.exe, exported a new Blender project, ran the NumCalc with " -istart x -iend x" parameters and got output .sofa files using the Python post-processing for a test project.

    Some observations:
    1 - in "Output2HRTF_Main.py" there is a bug on line:
    if not all(numpy.abs(frequencies[0] - numpy.diff(frequencies)) < .1):
    needs to be changed to:
    if not all(numpy.abs(numpy.diff(frequencies[0] - numpy.diff(frequencies))) < .1):

    2 - I get strange printouts from NumCalc (this project had only 1 out-of-4 frequency steps above 16.1kHz) but why is there a "highest allowable frequency"?:

    Warning: There are some frequencies higher than the highest allowable frequency!
    Number of frequencies outside the range= 1
    The highest allowable frequency = 16450.9
    

    3 - minor thing, but in NumCalc "NC_Main.cpp" on line 245 there is a strange commented out if statement. Seems to work anyway, but looks like it should not be left this way.

    4 - in "Output2HRTF_Main.py" on line:
    # -----------------------check number of freq bins----- the code really needs to be improved when there is time :)

    The NumCalcManager re-write is already 45% complete and will be ready very soon.

    Must say the new project structure is really impressively clean! And the Blender exporter also looks 40% less confusing than before. Great work!
    Now I need to figure out how to merge Left and Right side SOFA files into one :)

     
  • Sergejs

    Sergejs - 2022-01-19

    Correction: in "Output2HRTF_Main.py" line 300:
    if not all(numpy.abs(frequencies[0] - numpy.diff(frequencies)) < .1):
    is better to fix with:
    if any(numpy.abs(numpy.diff(frequencies, 2)) > .1) or frequencies[0] < 0.9:

     

    Last edit: Sergejs 2022-01-19
  • Sergejs

    Sergejs - 2022-01-19

    for robustness line 309 can be changer from
    fs = 2*frequencies[-1]
    to
    fs = round(2*frequencies[-1])

     
  • Fabian Brinkmann

    Hi Sergejs,

    thanks for the Feedback!

    • 1: fixed but used frequencies[0] < 0.1 instead of frequencies[0] < 0.9 -was there a specific reason for the 0.9?
    • 2: The limit is a rough estimation based on the triangles' edge lengths. As a rule of thumb there should be at least 6 edges per wave length. The estimation might be wrong for graded meshes with large elements away from the ear of interest. I think it was intended as a first place to look, if the results are weird.
    • 3: Did you have to uncomment the lines or did it work for you as it is?
    • 4: fixed - numFrequencies is now read from Info.txt and is read only once
     
  • Sergejs

    Sergejs - 2022-01-20

    Hi,

    1. 0.1 is perfectly fine, I just set it to allow for 1 Hz with some tolerance for rounding errors.
    2. this makes sense. As I used graded mesh with max 10 or 15mm edges during testing. Then this Warning should simply be ignored or someone could add extra clarification about graded meshed on line 1448 in NC_Input.cpp.
    3. I think no-one touched those lines in a long time and the code works both in Windows and Linux. Magic! I guess if anyone touches those lines NumCalc will completely break down :)
    4. Nice.

    BTW, now when Matlab post processing code is not even compatible with the rest of the changes, it would be best to comment out generation of Output2HRTF.m file. Today I accidentally tried to run it in Spyder and was very confused why everything stopped working :)

     
  • Fabian Brinkmann

    The Matlab version is kept up to date by someone in Vienna. I would only fully remove it , in case it won't be ready for the 1.0 release and live with it for now...

     
  • Sergejs

    Sergejs - 2022-02-07
    • status: open --> closed
     
  • Sergejs

    Sergejs - 2022-02-07

    We can close this - In fact the current Python workflow from /simplify_export_cpu_and_cores/ branch works great.

     

Log in to post a comment.