Free cookie consent management tool by TermsFeed Policy Generator

Opened 13 years ago

Closed 13 years ago

#1309 closed defect (done)

RealVectorAdditiveMoveWrapper is visible through the discovery mechanism

Reported by: cneumuel Owned by: swagner
Priority: medium Milestone: HeuristicLab 3.3.3
Component: PluginInfrastructure Version: 3.3.3
Keywords: Cc:

Description (last modified by abeham)

HeuristicLab.Problems.TestFunctions.RealVectorAdditiveMoveWrapper is not a storable class. It should be one!

Update: I still think it should not be storable, but I removed the default constructor to prevent discovery.

Change History (12)

comment:1 Changed 13 years ago by abeham

Why?

It does not live beyond the scope of the move evaluator's apply method and will thus not get serialized.

comment:2 Changed 13 years ago by cneumuel

When configuring a MetaOptimization parameter tree, it can be a possible operator of a parameter. When this parameter configuration tree is stored, those possible operators also need to be stored.

comment:3 Changed 13 years ago by abeham

The RealVectorAdditiveMoveWrapper is not an operator. I don't know what you're doing, but you shouldn't use this data type in any way.

Here is the scope where it lives:

protected override double Evaluate(double quality, RealVector point, AdditiveMove move) {
  RealVectorAdditiveMoveWrapper wrapper = new RealVectorAdditiveMoveWrapper(move, point);
  return AckleyEvaluator.Apply(wrapper);
}

It's not meant to be used in any other context.

comment:4 Changed 13 years ago by gkronber

I guess the problem is that RealVectorAdditiveMoveWrapper is a RealVector. Thus, automatic discovery of all possible values of a RealVector parameter also returns an instance of this class.

comment:5 Changed 13 years ago by abeham

How can I prevent such a discovery? Would it be enough to kill the default constructor (which doesn't make sense anyway) so that a default instance cannot be created?

comment:6 Changed 13 years ago by abeham

  • Status changed from new to accepted

I will remove the default constructor.

comment:7 Changed 13 years ago by abeham

  • Description modified (diff)
  • Milestone changed from HeuristicLab x.x.x to HeuristicLab 3.3.3
  • Owner changed from abeham to cneumuel
  • Status changed from accepted to reviewing
  • Summary changed from RealVectorAdditiveMoveWrapper is not storable to RealVectorAdditiveMoveWrapper is visible through the discovery mechanism

r5058

  • Removed default constructor to prevent discovery

Please check if this solved your problem, otherwise we'll make it Storable.

comment:8 Changed 13 years ago by cneumuel

  • Owner changed from cneumuel to abeham
  • Status changed from reviewing to assigned

The current solution throws an exception when the type is discovered. This can be verified with the following code:

var items = ApplicationManager.Manager.GetInstances(typeof(DoubleArray)).ToArray();

Exception: System.MissingMethodException: No parameterless constructor defined for this object.

The exception is thrown directly in LightweightApplicationManager.GetInstances and is uncaught:

    public IEnumerable<object> GetInstances(Type type) {
      return from t in GetTypes(type, true)
             select Activator.CreateInstance(t);
    }

Now, I certainly could catch the exception in my code, but this implementation would make it possible to break other plugins by introducing types without parameterless constructors. In my opinion either the ApplicationManager should catch the exception (1), or types which must not be discovered should be attributed with something like [Undiscoverable] (2). Or we just make RealVectorAdditiveMoveWrapper Storable (3).

(1) has the disadvantage that types which accidentally have no parameterless constructor can be introduced without any warning.

(2) would force the developer to think about the type discovery mechanism which i think is OK for types which should explicitly not be discovered.

(3) ok, if the scenario that a type should not be discoverable is really rare.

Last edited 13 years ago by cneumuel (previous) (diff)

comment:9 Changed 13 years ago by swagner

  • Owner changed from abeham to swagner
  • Status changed from assigned to accepted

comment:10 Changed 13 years ago by swagner

Implemented solution (1) in r5131. Solution has been reviewed by abeham and mkommend.

comment:11 Changed 13 years ago by swagner

  • Component changed from Problems.TestFunctions to PluginInfrastructure
  • Status changed from accepted to readytorelease

comment:12 Changed 13 years ago by mkommend

  • Resolution set to done
  • Status changed from readytorelease to closed
  • Version changed from 3.3.2 to 3.3.3
Note: See TracTickets for help on using tickets.