Skip to content

Refactor matching options #199

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

Open
wants to merge 14 commits into
base: master
Choose a base branch
from

Conversation

dumasl
Copy link
Contributor

@dumasl dumasl commented Mar 1, 2019

Color description

>>> import s2plib.stereo_matching_algo.stereo_matching as st
>>> st.StereoMatching.__subclasses__()
[]
>>> st.StereoMatching.stereo_methods_avail
{}
>>> import s2plib.stereo_matching_algo.mgm as mgm
>>> st.StereoMatching.__subclasses__()
[<class 'mgm.mgmMatching'>, <class 'mgm.mgm_multiMatching'>]
>>> st.StereoMatching.stereo_methods_avail
{'mgm': <class 'mgm.mgmMatching'>, 'mgm_multi': <class 'mgm_multi.mgmMatching'>}
>>>
>>>
>>>
>>> st.StereoMatching(mgm.mgmMatching)
<mgm.mgmMatching object at 0x2adca3c8feb8>
>>> st.StereoMatching('mgm')
<mgm.mgmMatching object at 0x2adc99c78ef0>

Technical summary

StereoMatching class

This MR refactors the way we use matching algorithms:

  • a new stereo_matching_algo package is created under s2plib module
  • a stereo_matching module can be found and it provides a StereoMatching class
  • also, mgm and msmw module are available with associated stereo matching classes

The StereoMatching class is

  • an abstract class created to force child classes to provide elementary methods with common arguments such as:
    • compute_disparity_map()
    • rectify_secondary_tile_only()
  • a metaclass as its constructor is overwritten to return the child class associated with the stereo method given as argument
    • nothing too fancy here but we overwrite __new__(), which is a constructor, and a constructor creates a class, and what creates a class is a metaclass, hence making StereoMatching a metaclass
    • this way there is no actual need for multiple control instructions to parse which stereo algorithm to instanciate.

StereoMatching itself does not provide any way to compute a disparity map. Subclasses are required for that purpose and shall be seen as stereo matching plugins.
This means importing StereoMatching does not provide any algorithm (let alone algorithms choice) if no plugin is loaded.

>>> import s2plib.stereo_matching_algo.stereo_matching as st
>>> st.StereoMatching.__subclasses__()
[]

StereoMatching plugins

Every time a plugin is loaded / imported with classes that are StereoMatching subclasses, it is registered as a StereoMatching class and then is instanciable.

>>> import s2plib.stereo_matching_algo.mgm as mgm
>>> st.StereoMatching.__subclasses__()
[<class 'mgm.mgmMatching'>, <class 'mgm.mgm_multiMatching'>]

Then it becomes possible to instanciate a stereo matcher:

>>> st.StereoMatching(mgm.mgmMatching)
<mgm.mgmMatching object at 0x2adca3c8feb8>

Now this does not really come handy since one has to know the class fullname (mgmMatching here). To ease this process and match s2p configuration (stereo matching algorithms), StereoMatching provides a decorator for self registring with a short name so that one can also istanciate the mgmMatching class like this:

>>> st.StereoMatching('mgm')
<mgm.mgmMatching object at 0x2adc99c78ef0>

The only thing requires then to use a new stereo matching algorithm (apart from implementing the stereo matching plugin) is to import it.

import s2plib.stereo_matching_algo.mgm

What changes for s2p

Then:

  • an instance of StereoMatching class is created inside s2p.py main method so that we crash early on in the process if the stereo method is not available
  • this instance is passed through parallel.call as extra args to rectification and stereo_matching s2p.py methods as both need it
  • since every stereo matcher might have specific args (as it already was the case with block_matching.py file), we use cfg as a kwargs since cfg is already a kwargs somehow
    • please note here that is would be nice to have a per step/algo key inside cfg so we can give a lighter cfg when relevant

Caution

This MR gets rid off lots of stereo matchers s2p provided so far. However, there is nothing preventing us from addind them back if required.

@dumasl dumasl changed the title WIP: Refactor matching options Refactor matching options Mar 1, 2019
@dumasl
Copy link
Contributor Author

dumasl commented Mar 1, 2019

Note that there still could be some improvements like:

  • making sure to define default values for optional & specific arguments (say 'census_ncc_win' for example)
  • adding every other stereo matching algorithms that were so far available

@dyoussef dyoussef requested review from gfacciol and carlodef March 4, 2019 12:41
@dyoussef dyoussef self-requested a review March 6, 2019 13:40
amiotc and others added 3 commits March 8, 2019 15:26
Environment export calls have been gather in a function, define
in the abstract metadata base class
…arams

Define default values for exported environment parameters
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

Successfully merging this pull request may close these issues.

4 participants