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

change ALOG to WLOG in some places #213

Closed
rkapl123 opened this issue Jan 22, 2024 · 2 comments
Closed

change ALOG to WLOG in some places #213

rkapl123 opened this issue Jan 22, 2024 · 2 comments

Comments

@rkapl123
Copy link
Contributor

rkapl123 commented Jan 22, 2024

Dear All!
As I'm using the alerts to signal/notify immediately actionable issues with the calculation run (results are wrong/not as expected), I'd like to ask for a change of this ALOG to WLOG, as in my opinion this doesn't constitute a condition that leads to wrong/unexpected results.
The removal is expectable (and reasonable!), so it should only be a warning.

void Portfolio::removeMatured(const Date& asof) {
    for (auto it = trades_.begin(); it != trades_.end(); /* manual */) {
        if ((*it).second->maturity() <= asof) {
            ALOG(StructuredTradeErrorMessage((*it).second, "", "Trade is Matured"));
            it=trades_.erase(it);
        } else {
            ++it;
        }
    }
}

Also in OREAnalytics\orea\scenario\scenariosimmarketparameters.cpp please also change the ALOGs to WLOGs

	if(!ccys.empty()) {
	    keys.insert(keys.end(), ccys.begin(), ccys.end());
            ALOG("ScenarioSimMarketParameters: SwaptionVolatilities/Currencies is deprecated, use Keys instead.");
        }
....
                if (!ccyAttr.empty()) {
                    key = ccyAttr;
                    ALOG("ScenarioSimMarketParameters: SwaptionVolatilities/Expiries: 'ccy' attribute is deprecated, "
                         "use 'key' instead.");
                }

Please also add information that not only the "Currencies" element should be changed to "Keys" but also the contained "Currency" elements to "Key", e.g. WLOG("ScenarioSimMarketParameters: SwaptionVolatilities/Currencies/Currency is deprecated, use SwaptionVolatilities/Keys/Key instead.");
This avoids some irritation.

In OREAnalytics\orea\app\analytics\xvaanalytic.cpp the alert also should regard the number of removed matured trades, as this is only useful to warn from accidental "losses" (by errors in trades):

        if (newPortfolio->size() < inputs_->portfolio()->size()) {
            ALOG("input portfolio size is " << inputs_->portfolio()->size() << 
                ", but we have built only " << newPortfolio->size() << " trades");
        }

-regards,
Roland

@rkapl123 rkapl123 changed the title change ALOG to WLOG in Portfolio::removeMatured change ALOG to WLOG in some places Jan 22, 2024
@rkapl123
Copy link
Contributor Author

I've created a PR for that in #216 . Concerning the trade removal message, I'm not sure, this is now a StructuredTradeErrorMessage, being thus on the same level with failed parsing from XML. I still don't think that this deserves such a strong error message, but removing trades needs other treatment as well (see above).

@rkapl123
Copy link
Contributor Author

I've also changed the structured trade error to a warning now as I think this doesn't deserve an alert. There is still the alert at the end of the analyses, when the portfolio sizes are compared, but this is more complicated.

@rkapl123 rkapl123 closed this as completed Jun 1, 2024
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

1 participant