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

CSGObject: Making *io, *parallel, *version singletons #2112

Open
tklein23 opened this issue Apr 8, 2014 · 9 comments
Open

CSGObject: Making *io, *parallel, *version singletons #2112

tklein23 opened this issue Apr 8, 2014 · 9 comments

Comments

@tklein23
Copy link
Contributor

tklein23 commented Apr 8, 2014

@karlnapf wrote:

@lambday I agree wiht you, singelton is a way cleaner concept than global variables. In fact, why not transition all of the global ones (IO, etc) to singleton? Any thoughts on that?
@lisitsyn @vigsterkr @sonney2k @iglesias ?

@tklein23
Copy link
Contributor Author

tklein23 commented Apr 8, 2014

@karlnapf - According to the principle of "seperation of concerns" this is a good idea. If the singletons are immutable, this is a very good idea, because it's indeed just encapsulating global data. (Using global variables would be fine as well. ;))

The the variables are mutable (like random or parameter framework), we'll get problem with threads.

If you're really trying to put this stuff into singletons, then please make a first draft and lets check if it's obfuscating the code too much.

@tklein23
Copy link
Contributor Author

tklein23 commented Apr 8, 2014

Asking simple questions:

  • why does an object need to maintain/store something like Parallel? A labels class for instance will never benefit from that.
  • why does an object need to maintain/store SGIO? As SGIO is always the same (?), why put it in every object?
  • putting Version in every object: can we run two different versions of shogun in parallel? Question inspired by @lisitsyn

@tklein23
Copy link
Contributor Author

tklein23 commented Apr 8, 2014

As @lisitsyn pointed out, the drawback of singletons are that you only have one. So we'll never be able again to run multiple shoguns!

@karlnapf
Copy link
Member

karlnapf commented Apr 9, 2014

I agree, drafting is very important for this! I guess one first step would also be to wait for @sonney2k comments since he wrote all this stuff (and he usually does not do things without having some thoughts)

Some of my thoughts:

  • Singleton drawbacks can easily addressed via putting the instances into a map if one needs more than one: http://en.wikipedia.org/wiki/Multiton_pattern
  • I agree, not all objects should have to store IO/parallel. In fact, it always irritates people that one can change behaviour globally via any instance. I always do this awkward GaussianKernel().io.set_log_level(1) thing in python

@lisitsyn
Copy link
Member

lisitsyn commented Apr 9, 2014

We can avoid storing IO, parallel and other things by adding some level of indirection here, which just aggregates these helper instances, like:


SGObject -> SG -> SGIO 

@tklein23
Copy link
Contributor Author

tklein23 commented Apr 9, 2014

@lisitsyn - are you talking about adding more classes to the hierarchy? We tried this (see SGRefObject and SGRefObjectArray) for improving the label classes but we (I?) got stuck somehow...

The problem is here, that we depend on SGObject everywhere and we need to touch every place (and possibly duplicate classes) to make it work for SGRefObjects as well.

Or did I miss your point? What do these arrows mean?

@lisitsyn
Copy link
Member

lisitsyn commented Apr 9, 2014

No, I just mean if we have multiple things composed with SGObject (SGIO, blabla) it makes sense to add an another layer here that composes all the helper classes. This way SGObject would have just one member, some SGContext or whatever you call it.

@tklein23
Copy link
Contributor Author

tklein23 commented Apr 9, 2014

@lisitsyn, Lets call it SGContext( Version, IO, Parallel ) a (const) SGContext * context in SGObject. So we can do sth. like context->get_version(), context->debug(), etc. Did I get this right? Does it reduce/increase the amount of (boiler plate) code?

Have a look on the CSGObject class: We have methods set_global_objects() here, which simply copy values from the global variables to the newly created object and increase ref counters. I think we wouldn't lose functionality or generality by "just" using the global one. This is the kind of an improvement I'm thinking of.

@lisitsyn
Copy link
Member

lisitsyn commented Apr 9, 2014

@tklein23 I think it decreases coupling so should be better

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants