noaa-ocs-hydrography / kluster

A distributed multibeam processing system built using the Pangeo ecosystem
Creative Commons Zero v1.0 Universal
46 stars 12 forks source link

Sonar model not understood 'em2040c' #114

Closed Kurtaja closed 7 months ago

Kurtaja commented 7 months ago

Hi and thank you for a great application! I've installed it on windows following this instruction, 'https://github.com/noaa-ocs-hydrography/kluster?tab=readme-ov-file#Installation' To process I use this python snippet: from HSTB.kluster.fqpr_intelligence import intel_process from HSTB.kluster.fqpr_convenience import convert_multibeam, process_multibeam, perform_all_processing from HSTB.kluster.fqpr_convenience import generate_new_surface fq = perform_all_processing(r"C:\Test\0322_20210513_084312.kmall", coord_system='WGS84', vert_ref='waterline') And it works fine for both .all and .kmall files collected with some random sonars. However, when I try with a file collected with a EM2040C head, the 'perform_all_processing' function give me this error ('intel_process' and 'convert_multibeam' give the same error), "NotImplementedError: Sonar model not understood "em2040c"", which appears to say that EM2040C has not been implemented. However, the README.md file contains this statement, "Kluster has been tested on:

ericgyounkin commented 7 months ago

Hi Kurt,

The issue is that for the longest time, the sonar model in the .all/.kmall file for the em2040c was 'em2045'. Here it appears to be 'em2040c'. https://github.com/noaa-ocs-hydrography/kluster/blob/master/HSTB/kluster/xarray_conversion.py#L34

Is this a new dataset? I'd be curious. Adding support wouldn't be too difficult, would you be able to share a small file?

Thanks, Eric

Kurtaja commented 7 months ago

Thanks for the rapid reply Eric, Sorry it took me a little while to get hold of some files I can share. Unfortunately I do not have a file small enough to upload on github, so have attached in this mail a we-transfer link, from where you can download them from instead. https://we.tl/t-3ElCPWR8e0 Kind regards Kurt

ericgyounkin commented 7 months ago

Hi Kurt, this is fixed in v1.1.6. You should be able to reinstall and get the fix now.

Kurtaja commented 7 months ago

Thank you very much for your prompt fix Eric. Much appreciated. I tried to install it in a new venv, using this instruction, 'https://github.com/noaa-ocs-hydrography/kluster?tab=readme-ov-file#Installation', and the installation did not report any errors, however, when I try to import these packages, "from HSTB.kluster.fqpr_intelligence import intel_process", I"m getting this error, "ModuleNotFoundError: No module named 'HSTB.resources'". Looking in 'site-packages\HSTB' in the venv, I have the 'drivers', 'kluster', 'shared' and 'versioning' modules, but not any resources module. Attached the requirement files for the old venv (that is working but not for EM2040C heads) and the new venv, that give me the error mentionned above, and I'm hence not able to get to work. I could not see that you have changed the import statments in your manuals. Could it be that you have forgotten to include that module in your egg? Anyhow, appreciate if you have the possibility to help me over this little bump :-). PS: I'm running on windows 11. requirements_new.txt requirements_old.txt

Kind regards Kurt

barry-gallagher commented 7 months ago

Hi @Kurtaja

I added the missing dependency to the setup files. I then installed with mamba+git (instead of conda) and it appears to work for me now. Two notes: I use Kluster inside our standard environment Pydro so didn't notice the additional dependency, second I tried mamba since it claims to be faster and conda stuck on some standard commands for me (not Kluster related) so I may have local a configuration issue with conda so please report any more issues you see.

barry-gallagher commented 7 months ago

As a quick follow up, I did get the conda install to work as well and will try and replace the release binary today or tomorrow.

Kurtaja commented 7 months ago

Hi Eric and Barry, I confirm that this now works on EM2040C in my end too. Thank you so much for your help! Kind regards Kurt