Inverse distortion #584
Inverse distortion #584
Conversation
Signed-off-by: Patrick Avery <[email protected]>
psavery
force-pushed
the
inverse-distortion
branch
3 times, most recently
from
5a1a659 to
26c10bb
Compare
Also update the conda build compiler to be C++ Signed-off-by: Patrick Avery <[email protected]>
psavery
force-pushed
the
inverse-distortion
branch
from
26c10bb to
daafa05
Compare
Signed-off-by: Patrick Avery <[email protected]>
This also adds a quick test to verify that it works properly. Signed-off-by: Patrick Avery <[email protected]>
|
Hello @ZackAttack614! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found: There are currently no PEP 8 issues detected in this Pull Request. Cheers! 🍻 Comment last updated at 2023-11-24 20:52:14 UTC |
I ran `black -l 80 -S ./test_inverse_distortion.py` Signed-off-by: Patrick Avery <[email protected]>
|
I should also test a couple of state files with these changes to verify that there are no issues. I believe this PR means that the transforms functions will now be compiled with a C++ compiler instead of a C compiler. In theory, this shouldn't cause any issues, but I will do a quick check to verify. After the comments have been addressed and a few state files tested, it will be good to merge! |
Signed-off-by: Patrick Avery <[email protected]>
NumpyToNativeEncoder is a custom JSON encoder that encodes numpy types into native types so it can be saved in JSON. There is no corresponding decoder, since the JSON just contains native types and does not need special decoding. Signed-off-by: Patrick Avery <[email protected]>
We can consider other encodings as well. Just need to get the bytes to a string and back. The raw_unicode_escale appears to work. Signed-off-by: Patrick Avery <[email protected]>
| with open( | ||
| os.path.join(test_dir, "data", "inverse_distortion_in_out.pkl"), 'rb' | ||
| os.path.join(test_dir, 'data', 'inverse_distortion_in_out.json'), | ||
| encoding='utf-8' |
There was a problem hiding this comment.
I think I shouldn't have to specify the encoding - it uses utf-8 by default.
There was a problem hiding this comment.
I tried this out with the single_GE example, and the total time went from 2 minutes and 36 seconds to 2 minutes with the branch!
|
I also tested a couple of state files and did not encounter any issues due to the fact that the compiler is C++ instead of C. |
|
At some point, we can verify that this produces a speed improvement for a real example with many grains, but it looks good, and we can verify that later. |
This PR focuses on the GE inverse distortion function. It has been rewritten into C++ and uses Eigen to do the heavy lifting of the matrix math.
In the multi-ruby example analysis running on my laptop, this PR takes the total runtime from 82 seconds to 62 seconds, cutting out roughly 25% of the runtime.