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

change Singleton / Pairwise to AbstractPrior / AbstractRelativeFactor #533

Closed
dehann opened this issue Jul 17, 2020 · 5 comments · Fixed by #537
Closed

change Singleton / Pairwise to AbstractPrior / AbstractRelativeFactor #533

dehann opened this issue Jul 17, 2020 · 5 comments · Fixed by #537

Comments

@dehann
Copy link
Member

dehann commented Jul 17, 2020

Or something similar, we can discuss here

Also see

@dehann
Copy link
Member Author

dehann commented Jul 17, 2020

@Affie
Copy link
Member

Affie commented Jul 17, 2020

The applicable code:

##==============================================================================
## Abstract Types
##==============================================================================

abstract type InferenceType end
abstract type PackedInferenceType end
abstract type FunctorInferenceType <: Function end
abstract type FactorOperationalMemory <: Function end
abstract type FunctorSingleton <: FunctorInferenceType end
abstract type FunctorPairwise <: FunctorInferenceType end
abstract type FunctorPairwiseMinimize <: FunctorInferenceType end

If a FunctorSingleton is always a prior, I would suggest 'Prior' then. It might be less of a learning curve to stick with familiar names? It took me a while to figure out what it was.

Also is the naming of Functor needed in FunctorInferenceType and FunctorSingleton
maybe it can be reduced to AbstractPriorFactor or even AbstractPrior and AbstractRelativeFactor ?

@dehann
Copy link
Member Author

dehann commented Jul 17, 2020

Yes, these two sound best to me -- let's do that:

AbstractPrior and AbstractRelativeFactor

Additional Info

Functor was previously introduced as part of an upgrade -- although I think the original Singleton and Pairwise stuff have long been deprecated. Dropping Functor should be fine and actually preferable.

@dehann dehann added this to the v0.9.0 milestone Jul 17, 2020
@dehann dehann changed the title change Singleton / Pairwise to AbsoluteData / RelativeData change Singleton / Pairwise to AbstractPrior / AbstractRelativeFactor Jul 17, 2020
@dehann dehann added the urgent label Jul 18, 2020
@GearsAD
Copy link
Collaborator

GearsAD commented Jul 19, 2020

Personally I like simpler/shorter, AbstractPrior and AbstractRelativeFactor sound like a plan

@dehann
Copy link
Member Author

dehann commented Jul 19, 2020

Lets do a deprecation cycle on the names because RoME and Caesar are both using Singleton and Pairwise abstracts at the moment. So basically just create the new ones next to it and place the old ones in the Deprecated.jl file.

GearsAD added a commit that referenced this issue Jul 19, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants