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

Unable to use a model specific Configuration #28

Open
rowtricker opened this issue Mar 7, 2017 · 3 comments
Open

Unable to use a model specific Configuration #28

rowtricker opened this issue Mar 7, 2017 · 3 comments

Comments

@rowtricker
Copy link

Currently, I am working on a project in which I am working with multiple column generation models. This makes that I need to be able to set requirements such as the generation of cuts in a flexible way, on a per model basis. Hence, I would like to adjust fields in the Configuration on a per model basis.

As far as I can tell from the current code, this is not possible. Is this correct?

@jkinable
Copy link
Collaborator

jkinable commented Mar 7, 2017

You currently can't do this.
Originally, I made several of the fields in Configuration final. The final attributes save guard from some weird behavior. Changing some parameters after the BranchAndPrice instance is created, may result in undefined behavior (e.g. changing the number of threads).
I can imagine that these limitations may be annoying.

Two possible solutions:

Solution 1. Have a look at how a logger like Logback handles multiple configuration files in the same project. We probably want to mimic what they are doing.

Solution 2. Add a method to Configuration to update a parameter. Although this is probably the easiest to implement, it may be quite hard to save guard from weird behavior. You don't want the user to change certain parameters after the Branch-and-Price instance has been created.

The configuration file was made static, such that each class has access to it, without passing it as a constructor argument.

Any suggestions here are welcome.

@rowtricker
Copy link
Author

rowtricker commented Mar 7, 2017

Preventing the parameters from changing during a solver run (CG vs B&P) indeed makes sense. However, that these cannot be changed in between models is indeed slightly annoying in some settings (especially when not forgetting about this restriction and being thrown an Exception).

Currently, I am using reflection as a workaround to change the fields, but having a more permanent solution would, of course, be better. Regarding the options:

Solution 1. I will look into this when I find some time. However, I am not sure if logging frameworks exactly face the same issue, as in their case some of the configurations can be changed on the fly. Moreover, I am somewhat afraid that these solutions will not be very simple.

Solution 2. This indeed also does not seem like a very good idea, especially as it is not really a long-term solution.

Some other simple options to consider are:

Solution 3. Relax the static access. Currently, it seems like the static getter is only used in 6 classes, meaning not too many parameters need to be added. This depends also on intended future use of this method.

Solution 4. Consider a local Configuration version and try to ensure that it is only changed from within the solver classes. So, pass a Configuration to a solver, which ensures that some SolverConfiguration is initialized and which additionally has a static method to get this configuration. This does not fully ensure immutability, but for example only allowing a package-private constructor there might make this somewhat less trivial. Not exactly sure how this would turn out, but it is relatively straightforward.

@rowtricker
Copy link
Author

rowtricker commented Mar 24, 2017

For completeness, hereby a workaround by means of reflection for a single field:

Configuration configuration2 = Configuration.getConfiguration();
try {
    Field f = configuration2.getClass().getDeclaredField("EXPORT_MODEL");
    f.setAccessible(true);
    f.setBoolean(configuration2, true);
} catch (NoSuchFieldException | IllegalAccessException e) {
         e.printStackTrace();
}

Of course, this should be used with care.

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

2 participants