Conversation
|
I have not figured out how to properly set the cell size and blanking distance for high resolution (HR) profiles, so tbd on that. |
|
Hi @jmcvey3 , thanks a lot for the great work. You are a legend! I will pull and test it hopefully next week. |
akeeste
left a comment
There was a problem hiding this comment.
Thanks @jmcvey3. This generally looks good to me. Do we need a new test to cover Aquadopp data?
@MRCGit-collab @MogoBRE if you can test this PR with your datasets that would be very helpful!
There was a problem hiding this comment.
These files are now ignored by .gitignore. I suggest we remove this file entirely
There was a problem hiding this comment.
Hold that thought - they're needed in test_data, but not the next directory up, where they're generated and not deleted if that test fails.
|
Tagging #356 for some of the original discussion |
Yes I need to write tests |
| units="1", | ||
| ), | ||
| "PressureMSB": _VarAtts( | ||
| "pressure": _VarAtts( |
There was a problem hiding this comment.
@jmcvey3, is it possible that changing these variable names would break any analysis code downstream for MHKiT users? IDK if it is worth having some deprecation strategy here or what makes sense. This could quickly get complex and difficult, but curious how you handle changes here more than anything.
| default_val=nan, | ||
| units="1", | ||
| long_name="Acoustic Signal Amplitude", | ||
| standard_name="signal_intensity_from_multibeam_acoustic_doppler_velocity_sensor_in_sea_water", |
There was a problem hiding this comment.
@jmcvey3 I think that adding standard names from the CF standard names table is a nice addition here. Do you think including the description would also be valuable?
Also, one nit here that I don't know how to solve is the sea_water part of the cf convention names. I think there are likely situations where the instrument is deployed in NOT sea water and that standard name may be incorrect and misleading. I don't have an obvious solution, just more of an observation here. I don't other non sea water variants signal_intensity_* in the cf standard name table.
|
Hi all,
Apologies for not replying earlier. I have been inundated with other work and lost track of this.
I pulled the PR and tried running it on my data set. I get a consistent error message:
self._inst = self.config.pop("config_type")
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
KeyError: 'config_type'
I'm no advanced user (and more of a field tech) and I checked with a few people but we couldn't figure out the issue.
So please excuse if this is a simple fix and a fault on my side.
I will continue to work on it, if I have time (it's still part of an ongoing project).
THanks in advance for all the effort!
Cheers,
Jürgen Zier
Marine Robotics Centre - MRC
T +32 (0)59-336229 / M +32 (0)468-293395
…________________________________
From: Adam Keester ***@***.***>
Sent: Tuesday, 24 February 2026 21:35
To: MHKiT-Software/MHKiT-Python ***@***.***>
Cc: Juergen Zier ***@***.***>; Mention ***@***.***>
Subject: Re: [MHKiT-Software/MHKiT-Python] Aquadopp (PR #434)
@akeeste approved this pull request.
Thanks @jmcvey3<https://github.com/jmcvey3>. This generally looks good to me. Do we need a new test to cover Aquadopp data?
@MRCGit-collab<https://github.com/MRCGit-collab> @MogoBRE<https://github.com/MogoBRE> if you can test this PR with your datasets that would be very helpful!
________________________________
On examples/data/dolfyn/test_data/AWAC_test01.dolfyn.log<#434 (comment)>:
These files are now ignored by .gitignore. I suggest we remove this file entirely
—
Reply to this email directly, view it on GitHub<#434 (review)>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/B5PB4DDWH63A2CODCNCDYA34NSYXPAVCNFSM6AAAAACUFILMHGVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZTQNJQGMYTMNJSG4>.
You are receiving this because you were mentioned.Message ID: ***@***.***>
|
|
Hi James,
Just a quick update, issue was on my side, I managed to run it and looks ok at first sight. Will do some further testing and give you an update.
Thanks a lot.
Juergen
Jürgen Zier
Marine Robotics Centre - MRC
T +32 (0)59-336229 / M +32 (0)468-293395
…________________________________
From: Juergen Zier ***@***.***>
Sent: Wednesday, 4 March 2026 14:13
To: MHKiT-Software/MHKiT-Python ***@***.***>; MHKiT-Software/MHKiT-Python ***@***.***>
Cc: Mention ***@***.***>
Subject: Re: [MHKiT-Software/MHKiT-Python] Aquadopp (PR #434)
Hi all,
Apologies for not replying earlier. I have been inundated with other work and lost track of this.
I pulled the PR and tried running it on my data set. I get a consistent error message:
self._inst = self.config.pop("config_type")
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
KeyError: 'config_type'
I'm no advanced user (and more of a field tech) and I checked with a few people but we couldn't figure out the issue.
So please excuse if this is a simple fix and a fault on my side.
I will continue to work on it, if I have time (it's still part of an ongoing project).
THanks in advance for all the effort!
Cheers,
Jürgen Zier
Marine Robotics Centre - MRC
T +32 (0)59-336229 / M +32 (0)468-293395
________________________________
From: Adam Keester ***@***.***>
Sent: Tuesday, 24 February 2026 21:35
To: MHKiT-Software/MHKiT-Python ***@***.***>
Cc: Juergen Zier ***@***.***>; Mention ***@***.***>
Subject: Re: [MHKiT-Software/MHKiT-Python] Aquadopp (PR #434)
@akeeste approved this pull request.
Thanks @jmcvey3<https://github.com/jmcvey3>. This generally looks good to me. Do we need a new test to cover Aquadopp data?
@MRCGit-collab<https://github.com/MRCGit-collab> @MogoBRE<https://github.com/MogoBRE> if you can test this PR with your datasets that would be very helpful!
________________________________
On examples/data/dolfyn/test_data/AWAC_test01.dolfyn.log<#434 (comment)>:
These files are now ignored by .gitignore. I suggest we remove this file entirely
—
Reply to this email directly, view it on GitHub<#434 (review)>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/B5PB4DDWH63A2CODCNCDYA34NSYXPAVCNFSM6AAAAACUFILMHGVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZTQNJQGMYTMNJSG4>.
You are receiving this because you were mentioned.Message ID: ***@***.***>
|
PR to add functionality to read Aquadopp data packets, but this code has only been tested on Aquadopp profilers (not single-point Aquadopps). Note that Aquadopp/AWAC waves functionality has also been added but is untested.
I've done a decent bit of cleanup of the nortek.py code in this PR as well.
The PR also simplifies the code related to the non-cabled ADV's "orientation_down" parameter. (This parameter exists because the z-axis is flipped on the non-cabled version of the ADV). This PR no longer reads the bit set in each burst ping, which is wont to oscillate for an unknown reason, but instead takes the value from the datafile's header. This means that DOLfYN assumes that a non-cabled ADV is not moved after it starts running.
Cabled ADVs, AWACs and Aquadopps also have the "orientation_down" parameter for user information, but it is not needed otherwise.