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

Enumeration of families #7

Closed
tnagler opened this issue Dec 23, 2016 · 13 comments
Closed

Enumeration of families #7

tnagler opened this issue Dec 23, 2016 · 13 comments

Comments

@tnagler
Copy link
Collaborator

tnagler commented Dec 23, 2016

For now, it's convenient to use the same numbering as he VineCopula package:

  • 0: Indendence
  • 1: Gauss
  • 2: Student
  • 3: Clayton
  • 4: Gumbel
  • 5: Frank
  • 6: Joe
  • ....

In the foreseeable future, we may want to change this, because we will include several families that are not part of VineCopula. I think it gets very confusing if we just take the smallest free integer whenever we implement a new family. For example, we could use the number as an indication of the number of parameters, like:

  • family_ < 100 nonparametric
  • 100 <= family_ < 200 one parameter
  • 200 <= family_ < 300 two parameters
  • and so one

Comments, suggestions?

@slayoo
Copy link
Collaborator

slayoo commented Feb 7, 2017

Well, probably it would be best not to need any integer representation (i.e., inheritance instead of conditional expressions) What are the current use cases where these integers are needed?

@tnagler
Copy link
Collaborator Author

tnagler commented Feb 7, 2017

The most important use case is to specify the set of families allowed for selection (this is something user's will do regularly). I think it's just more convenient to write {0, 1, 2, 3, 4, 5} instead of {Independence, Gaussian, Student, Clayton, Gumbel, Frank} (using an enum). We could also allow for both versions.

@slayoo
Copy link
Collaborator

slayoo commented Feb 8, 2017

I would still strongly vote for designing the API in a way that does not feature any integers/flags/enums. What about something like the skeleton below (excuse me if I'm missing some key point!):

#include <list>
#include <memory>
using namespace std;

struct copula_base
{
    virtual void magic() const = 0;
};

using ptr = unique_ptr<copula_base>;

struct copula_a : copula_base
{
    void magic() const {}
};

struct copula_b : copula_base
{
    void magic() const {}
};

void select(list<ptr> &copulae)
{
    for (auto &copula : copulae) copula->magic();
}

int main()
{
    select({
        ptr{new copula_a()},
        ptr{new copula_b()}
    });
}

@tnagler
Copy link
Collaborator Author

tnagler commented Feb 8, 2017

The key point is convenience for the user ;) But additionally, your version requires the family to be known at compile time, which isn't always the case.

Even if we keep the C++ library without any integers/enums, we will should introduce them for the R interface, because the average R user knows nothing about classes and inheritance (don't know about python). So we would need a wrapper that converts numbers to the inheritance style in any case (which is exactly what Bicop::create() does currently).

@slayoo
Copy link
Collaborator

slayoo commented Feb 8, 2017

Well, the user does not need to know anything about inheritance, my point is doing:

 select({ptr{new copula_a()}, ptr{new copula_b()} });

instead of

 select({44, 666});

The above is about the user perspective, but the actual goal would be not to have any conditional or switch statements in the library (if needed, they could indeed to to the R bindings; Python will certainly be fine with an class-based interface).

Concerning the compile-time requirement, IIUC this is merely the question of having the conditional/switch logic within the library or within the client code.

@tnagler
Copy link
Collaborator Author

tnagler commented Feb 8, 2017

OK, I think this would be feasible. In R we would certainly still use numbers. For python we should ask chris what he thinks is more convenient/accessible for the average python user (inheritance is also possible in R, but 90% of users would shy away from such a library).

@tvatter
Copy link
Collaborator

tvatter commented Mar 10, 2017

I personally like the integer representation. Additionally to the user friendliness, it's very convenient in the code.

@tvatter tvatter mentioned this issue Mar 10, 2017
19 tasks
@tnagler
Copy link
Collaborator Author

tnagler commented Mar 10, 2017

The initial question remains: how should we enumerate the families?

@tvatter
Copy link
Collaborator

tvatter commented Mar 10, 2017

I feel so comfortable working with the VineCopula encoding that I would be inclined in keeping 0 <= family_<= 10 as it is just by laziness. I personally like the idea of taking the smallest free integer whenever we implement a new family, but we should probably separate parametric and nonparametric copulas.

If you think that more structure is something we need, then I would argue in favor of an enumeration that uses either the class structure of the library or a category structure (rather than the specific parametrization as you suggested).

For instance:

  • family_ = 0 : independence
  • 1 < family_ < 50 : parametric
    • 1 < family_ < 10 : elliptical
    • 11 < family_ < 20 : archimedean
    • 21 < family_ < 30 : extreme-value
    • 31 < family_ < 40 : other
  • 51 <= family_ < 99 : nonparametric

@tnagler
Copy link
Collaborator Author

tnagler commented Mar 10, 2017

I don't like taking the next free integer at all. The category structure I like, although we'll need to think of how many families we have to reserve in each category (archimedean will need more than 10).

@slayoo
Copy link
Collaborator

slayoo commented Mar 12, 2017

One more think to take into account is probably serialisation of the vine structure - e.g. to handle save/restore functionality

@tnagler
Copy link
Collaborator Author

tnagler commented Mar 14, 2017

So we're going to abandon numbers and use enum class after all. Save/restore functionality is still an open issue.

@tvatter
Copy link
Collaborator

tvatter commented Mar 14, 2017

Since we decided to abandon numbers and use enum class, I'm closing the issue.

@tvatter tvatter closed this as completed Mar 14, 2017
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

3 participants