Changes made by Mark Snell to enable 4-beam Nortek Signature Parsing and bin mapping.
NOTE THAT THE UPDATES WILL REQUIRE MORE WORK AND TESTING PRIOR TO MERGING.
@ocehugo @sspagnol @evacougnon
Mark's notes are in the attached word document.
beam_compare.docx
Please contact Mark if you require more information or files for testing.
Notes from Hugo:
In summary, the new code is just changing some matrices indexes to account for the 4-beam (pretty much ~10 lines - 180-190 in beam2enu) but doing the calculation/access in different index/namespace. Hence the proposed changes are far from ideal since they are changing too much for too little - there should be only one way to perform the loops/access the index/incur the matmul. The conversion matrix being shaped/accessed differently for the two cases is jarringly confusing. The code has a lot of duplicated boilerplate between 3/4 beams. Hence, the code needs to be aligned and consistent between the cases handled.
Some extras will likely be needed:
1.The raw instrument file (Beam coordinates), the processed ones with the Nortek software, and references to the manual (page/equation/etc) and the mat files.
2.Tests for the transformation.
3.Tests for the binMapping of the different orientations.
I think Parser/Sig4beam_transform.m should be translated to a test (Pass/Fail) - you may use the function isequal_tol to verify the double-precision arguments up to whatever decimal precison you want and make the plots optional. This way, the code can fearlessly refactor all the code implemented. Unfortunately, the original beam2enu function will also need a test itself since there is nothing about it and any possible regressions will not be testable.
As always, all the above are my opinion about it 🙂. I'm not the gatekeeper anymore so others may have a different opinion. There are some extra points I noted too but they are minor in comparsion to the above.
PS: The changes in binMapping are likely wrong. The correct way here is probably to calculate the absolute value or reverse the dvar/beam variables given the shift in orientation. A modification here will require tests too to avoid/catch regressions.
Changes made by Mark Snell to enable 4-beam Nortek Signature Parsing and bin mapping. NOTE THAT THE UPDATES WILL REQUIRE MORE WORK AND TESTING PRIOR TO MERGING.
@ocehugo @sspagnol @evacougnon Mark's notes are in the attached word document. beam_compare.docx Please contact Mark if you require more information or files for testing.
Notes from Hugo: In summary, the new code is just changing some matrices indexes to account for the 4-beam (pretty much ~10 lines - 180-190 in beam2enu) but doing the calculation/access in different index/namespace. Hence the proposed changes are far from ideal since they are changing too much for too little - there should be only one way to perform the loops/access the index/incur the matmul. The conversion matrix being shaped/accessed differently for the two cases is jarringly confusing. The code has a lot of duplicated boilerplate between 3/4 beams. Hence, the code needs to be aligned and consistent between the cases handled.
Some extras will likely be needed:
1.The raw instrument file (Beam coordinates), the processed ones with the Nortek software, and references to the manual (page/equation/etc) and the mat files. 2.Tests for the transformation. 3.Tests for the binMapping of the different orientations.
I think
Parser/Sig4beam_transform.mshould be translated to a test (Pass/Fail) - you may use the functionisequal_tolto verify the double-precision arguments up to whatever decimal precison you want and make the plots optional. This way, the code can fearlessly refactor all the code implemented. Unfortunately, the original beam2enu function will also need a test itself since there is nothing about it and any possible regressions will not be testable.As always, all the above are my opinion about it 🙂. I'm not the gatekeeper anymore so others may have a different opinion. There are some extra points I noted too but they are minor in comparsion to the above.
PS: The changes in binMapping are likely wrong. The correct way here is probably to calculate the absolute value or reverse the
dvar/beamvariables given the shift in orientation. A modification here will require tests too to avoid/catch regressions.