Stricter behaviour of CalibratedCurves::createForwardCurve()

[NiklasRodi]

What is this PR for?

• CalibratedCurves::createForwardCurve(): do not create fallback curve if no curve with the given name exists in the model
• CalibratedCurves::createForwardCurve(): Only wrap discountCurve if it has not already been wrapped
• remove no superfluous flag isUseForwardCurve

What type of PR is it?

Feature

[cfries]

I do not understand the change. Maybe I have to look more closely,.... but:

• The Intention of CalibratedCurves::createForwardCurve() is
  • to create a forward curve and add it to the model. This should be done idempotent, that is, if the curve already exits it is not added again.
  • it offers two ways of creating a forward curve. The standard ForwardCurve or a combination of DiscountCurve + ForwardCurveFromDiscountCurve. The second way is an option is it is there only for testing (e.g. showing the issue with sensi-ozillations comming from this approach).
• With your change:
  • CalibratedCurves::createForwardCurve() does not add the curve to the model, hence it does not create a forward curve. The line model = model.addCurves(forwardCurve); has been removed.
  • The method just returns the name of the curve (your JavaDoc states "gets a forward curve").
  • The only functionality left is that it creates a wrapper if the model already contains a discount curve (which is a special case and maybe the reason why the code is still running).
  • The Exception IllegalArgumentException("Cannot create forwardCurve " + forwardCurveName + " as no such curve was found in the model... does not make sense to me. In case he cannot find a forward curve he should create a ForwardCurve?.

Confused.

(I though that the unit test should fail with this code, I have to check...)

[cfries]

Also: Why was

    private static final boolean isUseForwardCurve;
    static {
        // Default value is true
        isUseForwardCurve = Boolean.parseBoolean(System.getProperty("net.finmath.marketdata.calibration.CalibratedCurves.isUseForwardCurve","true"));
    }

removed. What about the situations where only likes to switch between the two approaches?

[NiklasRodi]

I don't like the feature that a forwardCurve is always created, even if the given forwardCurveName was not found in the model at all. What if I have already added a forwardCurve to my model that is almost "empty" but has e.g. a specific Interpolation method. Now I want to calibrate that curve but have a typo in the forwardCurveName such that it is not found in the model. Instead of getting an exception and being able to correct the name I will get some default forwardCurve that has not the interpolation method I wanted. This error will be very nasty to find. Maybe there are even worse problems.

I prefer the solution where the forwardCurve has to be created outside modulo maybe the wrapping feature (i.e. at least a Discount curve to base my forward curve on has to be created outside). I admit that the wrapping is the only feature left in the function (so maybe one should use a different name?). However, at one point I would also like to get rid of that internal wrapping (again it should be done outside) making the method totally redundant, i.e. at that point I would delete it for good.

[NiklasRodi]

• The only appearance of isUseForwardCurve is in the creation of a forwardCurve - the feature that I have removed. If you agree with that then this parameter is redundant, so I removed it
• The line "model = model.addCurves(forwardCurve)" is still there - I moved it to the only case where imO we really need to add a curve (where we wrap a discountCurve which has not been wrapped before)
• I need the feature that the method returns the Name of the forwardCurve as it may have been changed because of the wrapping
• The exception is, as I have said, my intenion

[cfries]

OK. Now I understand your intention. But...

• it changes the known behaviour of CalibratedCurves - hence would break some Spreadsheets (where the class is used) and
• I believe that the method may be the wrong place.

... let me check....

[NiklasRodi]

• If you want I can create a flag "createDefaultCurvesIfNecessary" with default=true that I can set to false for my purposes. However, in the long-run I think one should get rid of this default curve creation as it is very dangerous. Also it would be very simple to make the spreadsheets run again
• What do you mean by "the method may be the wrong place". Wrong place for what?

[NiklasRodi]

Closed as (partially) implemented in commit d2c95c4