Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

No "add_custom_element" function in kWaveArray #208

Closed
bvale1 opened this issue Sep 13, 2023 · 7 comments
Closed

No "add_custom_element" function in kWaveArray #208

bvale1 opened this issue Sep 13, 2023 · 7 comments

Comments

@bvale1
Copy link
Contributor

bvale1 commented Sep 13, 2023

There appears to be no "add_custom_element" function in the kWaveArray class which should allow the user to define their own integration points for a custom array element

@bvale1
Copy link
Contributor Author

bvale1 commented Sep 15, 2023

I've added this function in my own branch of k-wave-python and started using it, happy to write a test as well and submit a pull request :)

@waltsims
Copy link
Owner

Hey!

Thanks for looking into this. A PR would be great. Just in time for our planned v0.3.0 release.

Best,
Walter

@bvale1
Copy link
Contributor Author

bvale1 commented Sep 16, 2023

I've added a few lines to KWaveArray_test.py that should test the add_custom_element function but I'm not entirely sure how to run the tests, could you assist?

@waltsims
Copy link
Owner

Hey,

The pattern we use is to write a dataCollector where we save artifacts in Matlab of a running example.

For you this would mean adding some test examples to collect_kWaveArray that makes a custom element. Then in the respective python file kWaveArray_test.py updating the python code to execute the same operations and assert equality between the outputs.

For this pattern, we use the helper classes, TestRecorder in Matlab, and TestRecordReader in python.

To run the test suite locally, one can run run_all_collectors in MATLAB and then run the python test_suite with pytest.

If you open a PR on your branch, this will also be done in the CI to test your code. This might be helpful if you don't have MATLAB installed.

Thanks for contributing to the project, and I hope this helps. If something is not clear, let me know.

Best,
Walter

@bvale1
Copy link
Contributor Author

bvale1 commented Sep 19, 2023

Hi Walter,

I've written a couple of cases (one for 2d and one for 3d) I've just had some issues running them, I changed the Dockerfile from 'COPY README.md .' to 'COPY docs docs/' in order to build it on my machine. I currently have a problem running the collectors in MATLAB 'unrecognized function or variable focusedBowlONeil'. I can have another crack at it tomorrow :)

Best wishes,
Billy.

@waltsims
Copy link
Owner

Thanks for updating the DockerFile. I last used it quite some time ago. Please add those changes to your PR!

@waltsims
Copy link
Owner

closed with #212

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants