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.

Sign up for GitHub

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

Jump to bottom

Simplify IRISPipeline output. #12

Merged
merged 3 commits into from
Mar 8, 2024
Merged

Simplify IRISPipeline output. #12

merged 3 commits into from
Mar 8, 2024

Conversation

wiktorlazarski
Copy link
Member

Simplify IRISPipeline output.

PR description

Currently, IRISPipeline returns serialised version of generated IrisTemplate that causes confusions and make it less intuitive how one can use HammingDistanceMatcher. This PR simplifies IRISPipeline output such that together with metadata, unserialised IrisTemplate object is by default returned from IRISPipeline inference call.

Also, update ruff tool setup to remove it's warnings and apply code quality tools to unify code base style etc.

Issue

Without knowing more details about inner working of a code base, users find it difficult to simply take two IR images, generate IrisTemplates and use HammingDistanceMatcher to get final distance between two irises.

Solution

Introduce a new builder function that returns IrisTemplate object (if error wasn't raised during the IRISPipeline inference call) and metadata in a dictionary.

Type

  • Feature
  • Refactoring

Checklist

  • I've made sure that my code works as expected by writing unit tests.
  • I've checked if my code doesn't generate warnings or errors.
  • I've performed a self-review of my code.
  • I've made sure that my code follows the style guidelines of the project.
  • I've commented hard-to-understand parts of my code.
  • I've made appropriate changes in the documentation.

@wiktorlazarski wiktorlazarski added the enhancement New feature or request label Mar 6, 2024
@wiktorlazarski wiktorlazarski self-assigned this Mar 6, 2024
@wiktorlazarski wiktorlazarski requested a review from a team as a code owner March 6, 2024 13:17
ycbiometrics
ycbiometrics reviewed Mar 6, 2024
View reviewed changes
Copy link
Contributor

@ycbiometrics ycbiometrics left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the change. I think any effort to try to simply the open-source while maintaining the functionality will be greatly appreciated.

colab/GettingStarted.ipynb Outdated
"\n",
"Each code available in `output[\"iris_template\"]` dictionary is a `numpy.ndarray` of shape `(16, 256, 2, 2)`. The output shape of iris code is determined by `IRISPipeline` filter bank parameters. The iris/mask code shape's dimmensions correspond to the following `(iris_code_height, iris_code_width, num_filters, 2)`. Values `iris_code_height` and `iris_code_width` are determined by `ProbeSchema`s defined for `ConvFilterBank` object and `num_filters` is determined by number of filters specified for `ConvFilterBank` object. The last `2` value of the iris/mask code dimmension corresponds to real and complex parts of each complex filter response.\n",
"Each code available in `output[\"iris_template\"]` object is a `numpy.ndarray` of shape `(16, 256, 2)`. The length of arrays containing iris codes and mask codes is determined by `IRISPipeline` filter bank parameters. The iris/mask code shape's dimmensions correspond to the following `(iris_code_height, iris_code_width, 2)`. Values `iris_code_height` and `iris_code_width` are determined by `ProbeSchema`s defined for `ConvFilterBank` object and `num_filters` is determined by number of filters specified for `ConvFilterBank` object. The last `2` value of the iris/mask code dimmension corresponds to real and complex parts of each complex filter response.\n",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"to the real and imaginary parts of each complex filter response", "are determined by ProbeSchema's definition for ..."

docs/source/examples/getting_started.rst Outdated

Each code available in ``output["iris_template"]`` dictionary is a ``numpy.ndarray`` of shape ``(16, 256, 2, 2)``. The output shape of iris code is determined by ``IRISPipeline`` filter bank parameters. The iris/mask code shape's dimensions correspond to the following ``(iris_code_height, iris_code_width, num_filters, 2)``. Values ``iris_code_height`` and ``iris_code_width`` are determined by ``ProbeSchema``s defined for ``ConvFilterBank`` object and ``num_filters`` is determined by number of filters specified for ``ConvFilterBank`` object. The last ``2`` value of the iris/mask code dimension corresponds to real and complex parts of each complex filter response.
Each code available in ``output["iris_template"]`` dictionary is a ``numpy.ndarray`` of shape ``(16, 256, 2)``. The length of arrays containing iris codes and mask codes is determined by ``IRISPipeline`` filter bank parameters. The iris/mask code shape's dimensions correspond to the following ``(iris_code_height, iris_code_width, 2)``. Values ``iris_code_height`` and ``iris_code_width`` are determined by ``ProbeSchema``s defined for ``ConvFilterBank`` object and ``num_filters`` is determined by number of filters specified for ``ConvFilterBank`` object. The last ``2`` value of the iris/mask code dimension corresponds to real and complex parts of each complex filter response.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same here: "to the real and imaginary parts of each complex filter response", "are determined by ProbeSchema's definition for ..."

TanguyJeanneau
TanguyJeanneau requested changes Mar 8, 2024
View reviewed changes
Copy link
Contributor

@TanguyJeanneau TanguyJeanneau left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the contribution ! One minor comment on simplifying more build_simple_output, then it will be all good to go 🙌

src/iris/orchestration/output_builders.py Outdated
Comment on lines 30 to 35
elif isinstance(exception, Exception):
error = {
"error_type": type(exception).__name__,
"message": str(exception),
"traceback": "".join(traceback.format_tb(exception.__traceback__)),
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@wiktorlazarski This is already dealt with in __get_error, we can simplify this function even more 🙂

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice! I completely missed that. Thanks for the hint. That'll allow us to simplify also build_orb_output function.

TanguyJeanneau
TanguyJeanneau approved these changes Mar 8, 2024
View reviewed changes
Copy link
Contributor

@TanguyJeanneau TanguyJeanneau left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM ! 🙌 🌲

@wiktorlazarski wiktorlazarski merged commit 9149d1e into dev Mar 8, 2024
5 checks passed
@wiktorlazarski wiktorlazarski deleted the wiktorlazarski/simplify-iris-pipeline-output branch March 8, 2024 16:32
@TanguyJeanneau TanguyJeanneau mentioned this pull request Jun 17, 2024
Merged
9 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@TanguyJeanneau TanguyJeanneau TanguyJeanneau approved these changes

@ycbiometrics ycbiometrics Awaiting requested review from ycbiometrics

Assignees

@wiktorlazarski wiktorlazarski

Labels
enhancement New feature or request
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@wiktorlazarski @TanguyJeanneau @ycbiometrics