Re: [QuantLib-svn] SF.net SVN: quantlib:[17128] trunk/QuantLib/ql/cashflows/cashflows.cpp

classic Classic list List threaded Threaded
3 messages Options
Reply | Threaded
Open this post in threaded view
|

Re: [QuantLib-svn] SF.net SVN: quantlib:[17128] trunk/QuantLib/ql/cashflows/cashflows.cpp

Luigi Ballabio
On Fri, 2010-02-19 at 11:36 +0000, [hidden email] wrote:
> Revision: 17128
>           http://quantlib.svn.sourceforge.net/quantlib/?rev=17128&view=rev
> Author:   nando
> Date:     2010-02-19 11:36:03 +0000 (Fri, 19 Feb 2010)
>
> Log Message:
> -----------
> ugly unreasonable patch, help would be appreciated

Try using reverse iterators.

Luigi


--

Don't let school get in the way of your education.
-- Mark Twain



------------------------------------------------------------------------------
Download Intel® Parallel Studio Eval
Try the new software tools for yourself. Speed compiling, find bugs
proactively, and fine-tune applications for parallel performance.
See why Intel Parallel Studio got high marks during beta.
http://p.sf.net/sfu/intel-sw-dev
_______________________________________________
QuantLib-dev mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/quantlib-dev
Reply | Threaded
Open this post in threaded view
|

Re: [QuantLib-svn] SF.net SVN: quantlib:[17128] trunk/QuantLib/ql/cashflows/cashflows.cpp

Ferdinando M. Ametrano-3
Hi Luigi

On Mon, Feb 22, 2010 at 9:36 AM, Luigi Ballabio <[hidden email]> wrote:
> Try using reverse iterators.

following your suggestion I found out a bug in the previousAmount and previousRate functions, which was easily fixed switching to reverse iterator (please find the patch below).

Unfortunately the fix is not backward compatible as it requires CashFlows::previousCashFlow and BondFunctions::previousCashFlow to return a Leg::const_reverse_iterator instead of Leg::const_iterator. I know you just tagged 1.0, but this fix might be worth porting to 1.0 before release. What is your opinion ?

ciao -- Nando

PS If you go for fixing the 1.0, less relevant backward-compatible bug-fix Rev17144 and Rev17143 can be ported too

============ patch ===============================================


Index: ql/cashflows/cashflows.cpp
===================================================================
--- ql/cashflows/cashflows.cpp (revision 17128)
+++ ql/cashflows/cashflows.cpp (working copy)
@@ -81,29 +81,22 @@
        return true;
    }

-    Leg::const_iterator
+    Leg::const_reverse_iterator
    CashFlows::previousCashFlow(const Leg& leg,
                                bool includeSettlementDateFlows,
                                Date settlementDate) {
        if (leg.empty())
-            return leg.end();
+            return leg.rend();

        if (settlementDate == Date())
            settlementDate = Settings::instance().evaluationDate();

-        if ( ! (*leg.begin())->hasOccurred(settlementDate,
-                                           includeSettlementDateFlows) )
-            return leg.end();
-
-        Leg::const_iterator i = nextCashFlow(leg,
-                                             includeSettlementDateFlows,
-                                             settlementDate);
-        // --i is not what we're looking for since there
-        // might be more than one CashFlow at (*--i)->date()
-        Date beforeLastPaymentDate = (*--i)->date()-1;
-        return nextCashFlow(leg,
-                            includeSettlementDateFlows,
-                            beforeLastPaymentDate);
+        Leg::const_reverse_iterator i;
+        for (i = leg.rbegin(); i<leg.rend(); ++i) {
+            if ( (*i)->hasOccurred(settlementDate, includeSettlementDateFlows) )
+                return i;
+        }
+        return leg.rend();
    }

    Leg::const_iterator
@@ -127,61 +120,52 @@
    Date CashFlows::previousCashFlowDate(const Leg& leg,
                                         bool includeSettlementDateFlows,
                                         Date settlementDate) {
-        Leg::const_iterator cf = previousCashFlow(leg,
-                                                  includeSettlementDateFlows,
-                                                  settlementDate);
-        if (cf==leg.end()) return Date();
+        Leg::const_reverse_iterator cf;
+        cf = previousCashFlow(leg, includeSettlementDateFlows, settlementDate);
+
+        if (cf==leg.rend())
+            return Date();
+
        return (*cf)->date();
    }

    Date CashFlows::nextCashFlowDate(const Leg& leg,
                                     bool includeSettlementDateFlows,
                                     Date settlementDate) {
-        Leg::const_iterator cf = nextCashFlow(leg,
-                                              includeSettlementDateFlows,
-                                              settlementDate);
-        if (cf==leg.end()) return Date();
+        Leg::const_iterator cf;
+        cf = nextCashFlow(leg, includeSettlementDateFlows, settlementDate);
+
+        if (cf==leg.end())
+            return Date();
+
        return (*cf)->date();
    }

    Real CashFlows::previousCashFlowAmount(const Leg& leg,
                                           bool includeSettlementDateFlows,
                                           Date settlementDate) {
-        Leg::const_iterator cf = previousCashFlow(leg,
-                                                  includeSettlementDateFlows,
-                                                  settlementDate);
-        if (cf==leg.end()) return Real();
+        Leg::const_reverse_iterator cf;
+        cf = previousCashFlow(leg, includeSettlementDateFlows, settlementDate);

+        if (cf==leg.rend())
+            return Real();
+
        Date paymentDate = (*cf)->date();
        Real result = 0.0;
-
-        // when cf==leg.begin() the following code crashes at --cf
-        // on VC8/9 Debug (boundary check)
-        // but it also crashes in Release mode as if it would evaluate
-        // (*cf) even when cf<leg.begin()
-        //
-        // help or suggestion would be appreciated
-        //
-        //for (; cf>=leg.begin() && (*cf)->date()==paymentDate; --cf)
-        //    result += (*cf)->amount();
-
-        // ugly patch...
-        for (; cf>leg.begin() && (*cf)->date()==paymentDate; --cf)
+        for (; cf<leg.rend() && (*cf)->date()==paymentDate; ++cf)
            result += (*cf)->amount();
-        if (cf==leg.begin() && (*cf)->date()==paymentDate)
-            result += (*cf)->amount();
-
        return result;
    }

    Real CashFlows::nextCashFlowAmount(const Leg& leg,
                                       bool includeSettlementDateFlows,
                                       Date settlementDate) {
-        Leg::const_iterator cf = nextCashFlow(leg,
-                                              includeSettlementDateFlows,
-                                              settlementDate);
-        if (cf==leg.end()) return Real();
+        Leg::const_iterator cf;
+        cf = nextCashFlow(leg, includeSettlementDateFlows, settlementDate);

+        if (cf==leg.end())
+            return Real();
+
        Date paymentDate = (*cf)->date();
        Real result = 0.0;
        for (; cf<leg.end() && (*cf)->date()==paymentDate; ++cf)
@@ -192,18 +176,20 @@
    // Coupon utility functions
    namespace {

+        template<typename Iter>
        Rate aggregateRate(const Leg& leg,
-                           Leg::const_iterator cf) {
-            if (cf==leg.end()) return 0.0;
+                           Iter first,
+                           Iter last) {
+            if (first==last) return 0.0;

-            Date paymentDate = (*cf)->date();
+            Date paymentDate = (*first)->date();
            bool firstCouponFound = false;
            Real nominal = 0.0;
            Time accrualPeriod = 0.0;
            DayCounter dc;
            Rate result = 0.0;
-            for (; cf<leg.end() && (*cf)->date()==paymentDate; ++cf) {
-                shared_ptr<Coupon> cp = dynamic_pointer_cast<Coupon>(*cf);
+            for (; first<last && (*first)->date()==paymentDate; ++first) {
+                shared_ptr<Coupon> cp = dynamic_pointer_cast<Coupon>(*first);
                if (cp) {
                    if (firstCouponFound) {
                        QL_REQUIRE(nominal       == cp->nominal() &&
@@ -221,7 +207,7 @@
                }
            }
            QL_ENSURE(firstCouponFound,
-                      "next cashflow (" << paymentDate << ") is not a coupon");
+                      "no coupon paid at cashflow date " << paymentDate);
            return result;
        }

@@ -230,19 +216,18 @@
    Rate CashFlows::previousCouponRate(const Leg& leg,
                                       bool includeSettlementDateFlows,
                                       Date settlementDate) {
-        Leg::const_iterator cf = previousCashFlow(leg,
-                                                  includeSettlementDateFlows,
-                                                  settlementDate);
-        return aggregateRate(leg, cf);
+        Leg::const_reverse_iterator cf;
+        cf = previousCashFlow(leg, includeSettlementDateFlows, settlementDate);
+
+        return aggregateRate<Leg::const_reverse_iterator>(leg, cf, leg.rend());
    }

    Rate CashFlows::nextCouponRate(const Leg& leg,
                                   bool includeSettlementDateFlows,
                                   Date settlementDate) {
-        Leg::const_iterator cf = nextCashFlow(leg,
-                                              includeSettlementDateFlows,
-                                              settlementDate);
-        return aggregateRate(leg, cf);
+        Leg::const_iterator cf;
+        cf = nextCashFlow(leg, includeSettlementDateFlows, settlementDate);
+        return aggregateRate<Leg::const_iterator>(leg, cf, leg.end());
    }

    Date CashFlows::accrualStartDate(const Leg& leg,
Index: ql/cashflows/cashflows.hpp
===================================================================
--- ql/cashflows/cashflows.hpp (revision 17127)
+++ ql/cashflows/cashflows.hpp (working copy)
@@ -55,7 +55,7 @@
        //! \name CashFlow functions
        //@{
        //! the last cashflow paying before or at the given date
-        static Leg::const_iterator
+        static Leg::const_reverse_iterator
        previousCashFlow(const Leg& leg,
                         bool includeSettlementDateFlows,
                         Date settlementDate = Date());
Index: ql/pricingengines/bond/bondfunctions.cpp
===================================================================
--- ql/pricingengines/bond/bondfunctions.cpp (revision 17144)
+++ ql/pricingengines/bond/bondfunctions.cpp (working copy)
@@ -47,8 +47,9 @@
        return bond.notional(settlement)!=0.0;
    }

-    Leg::const_iterator BondFunctions::previousCashFlow(const Bond& bond,
-                                                        Date settlement) {
+    Leg::const_reverse_iterator
+    BondFunctions::previousCashFlow(const Bond& bond,
+                                    Date settlement) {
        if (settlement == Date())
            settlement = bond.settlementDate();

Index: ql/pricingengines/bond/bondfunctions.hpp
===================================================================
--- ql/pricingengines/bond/bondfunctions.hpp (revision 17127)
+++ ql/pricingengines/bond/bondfunctions.hpp (working copy)
@@ -60,8 +60,9 @@

        //! \name CashFlow inspectors
        //@{
-        static Leg::const_iterator previousCashFlow(const Bond& bond,
-                                                    Date refDate = Date());
+        static Leg::const_reverse_iterator
+        previousCashFlow(const Bond& bond,
+                         Date refDate = Date());
        static Leg::const_iterator nextCashFlow(const Bond& bond,
                                                Date refDate = Date());
        static Date previousCashFlowDate(const Bond& bond,




------------------------------------------------------------------------------
Download Intel&#174; Parallel Studio Eval
Try the new software tools for yourself. Speed compiling, find bugs
proactively, and fine-tune applications for parallel performance.
See why Intel Parallel Studio got high marks during beta.
http://p.sf.net/sfu/intel-sw-dev
_______________________________________________
QuantLib-dev mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/quantlib-dev
Reply | Threaded
Open this post in threaded view
|

Re: [QuantLib-svn] SF.net SVN: quantlib:[17128] trunk/QuantLib/ql/cashflows/cashflows.cpp

Luigi Ballabio
On Mon, 2010-02-22 at 20:05 +0100, Ferdinando Ametrano wrote:

> On Mon, Feb 22, 2010 at 9:36 AM, Luigi Ballabio
> <[hidden email]> wrote:
> > Try using reverse iterators.
>
> following your suggestion I found out a bug in the previousAmount and
> previousRate functions, which was easily fixed switching to reverse
> iterator (please find the patch below).
>
> Unfortunately the fix is not backward compatible as it requires
> CashFlows::previousCashFlow and BondFunctions::previousCashFlow to
> return a Leg::const_reverse_iterator instead of Leg::const_iterator.

Well, it doesn't strictly require it---you could retrieve the iterator
from the reverse iterator.  But I see that the common use case would be
to turn it into a reverse itereator again, so it makes more sense to
change the signature.

<sigh> Ok, I'm applying it. Commit the thing on the trunk asap.

Luigi


--

Do the right thing. It will gratify some people and astonish the rest.
-- Mark Twain



------------------------------------------------------------------------------
Download Intel&#174; Parallel Studio Eval
Try the new software tools for yourself. Speed compiling, find bugs
proactively, and fine-tune applications for parallel performance.
See why Intel Parallel Studio got high marks during beta.
http://p.sf.net/sfu/intel-sw-dev
_______________________________________________
QuantLib-dev mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/quantlib-dev