does this seem like the most reasonable place to filter out short positions as soon as the portfolio is imported? will this fix the issue in #463?

also @FrederickFa be aware that this is intended to fix the web tool scripts way of doing things, but I don't know if something else needs to be done to fix the other ways of running PACTA that are still lurking around

@FrederickFa maybe something equivalent needs to happen in here?https://github.com/2DegreesInvesting/PACTA_analysis/blob/abf4ef9acd22347994e193c8fef8baf537a4398d/0_portfolio_input_check_functions.R#L14-L48

This is why I didn't do this earlier... it didn't fall off my radar, I just don't know the proper how/what/where.

I understand, but leaving it as it is now is in my opinion definitely worse than filtering them.

It is also interesting that this is not documented anywhere. At least I could not find any information about our communication on short positions either in the PACTA COP documentation or in the Toolbox.

This definitely needs to have a methodological review, and a more permanent decision, but this looks like a good fix for now.

@AlexAxthelm So you see rather no risk with the points I mentioned when filtering at the very beginning?

Also, this PR doesn't solve the issue within the offline workflow. Do you envision a separate PR then @cjyetman @AlexAxthelm

So you see rather no risk with the points I mentioned when filtering at the very beginning?

No, the risks you brought up are still relevant. But in the absence of any better suggestions, I think we should go with the behavior you suggested in the discussion on #463:

I would also propose to only filter position above zero market value.

I am assuming that your position on the solution hasn't changed, and since you're flagging this as a major issue, we need to get a solution that is functional (if not perfect).

Also, this PR doesn't solve the issue within the offline workflow.

It seems as though each analyst has their own different "offline" workflow at this point, so until we have consolidated central structure that we can get everyone to agree upon, I don't think we can support those beyond offering changes to get_input_files(). If someone is willing to bypass that function, then they are taking on the responsibility for maintaining their different workflow themselves.

So you see rather no risk with the points I mentioned when filtering at the very beginning?

No, the risks you brought up are still relevant. But in the absence of any better suggestions, I think we should go with the behavior you suggested in the discussion on #463:

But the behavior I suggested in #463 is different from the proposed changes in this PR.

The change in this PR kicks out all short positions completely from the entire PACTA, hence we interfere with the portfolio structure.

My proposal is to only filter short positions within the analysis part, hence avoiding any weird negative production values. But the portfolio itself remains untouched.

I would also propose to only filter position above zero market value.

I am assuming that your position on the solution hasn't changed, and since you're flagging this as a major issue, we need to get a solution that is functional (if not perfect).

As the proposed change in this PR is market_value > 0 , this would be fine from my side.

Also, this PR doesn't solve the issue within the offline workflow.

It seems as though each analyst has their own different "offline" workflow at this point, so until we have consolidated central structure that we can get everyone to agree upon, I don't think we can support those beyond offering changes to get_input_files(). If someone is willing to bypass that function, then they are taking on the responsibility for maintaining their different workflow themselves.

Ok. So in case for MFM, we still use the workflow consisting of 1_portfolio_check_initialisation.R, 2_project_input_analysis.R and 3_run_analysis.R, and update our repositories for new updates. I would have thought that this is the only offline workflow.

Therefore, when going back to #463, I would have proposed a change to 3_run_analysis.R: to filter short positions from getting included in the analysis.

But the behavior I suggested in #463 is different from the proposed changes in this PR.

I see the difference now. thank you for explaining. I'll be continuing this discussion on ADO. AB 661

Putting this in draft mode since there is still no agreed upon solution according to PBI 661

My proposal is to only filter short positions within the analysis part, hence avoiding any weird negative production values. But the portfolio itself remains untouched.

@FrederickFa if you can be more precise about what/where this means, I'll take another stab at it. You should be aware though that this so-called "offline" version of PACTA has basically been abandoned and not maintained for many months now, so at some point its results may diverge from those that the web tool scripts produce.

To be honest, I'm not sure how to interpret all the comments in PRODUCT BACKLOG ITEM 661, so I'm not 100% sure there is agreement on this.

Thanks @cjyetman for picking this up!

To be honest, I'm not sure how to interpret all the comments in PRODUCT BACKLOG ITEM 661, so I'm not 100% sure there is agreement on this

I think there is still no agreement but rather a set of possible solutions. As leaving the current situation is not a good idea, I guess the best way in the short term is to be pragmatic and just filter out short position in the analysis parts. So, maybe add the following code after the mentioned lines:

• Filter out short positions in equity portfolio:
  https://github.com/2DegreesInvesting/PACTA_analysis/blob/477da47bbc2c45730bea051f5850b39f1fb24e5b/3_run_analysis.R#L54
  port_eq <- port_eq %>% filter(port_weight > 0)

• Filter out short positions in bonds portfolio:
  https://github.com/2DegreesInvesting/PACTA_analysis/blob/477da47bbc2c45730bea051f5850b39f1fb24e5b/3_run_analysis.R#L154
  port_cb <- port_cb %>% filter(port_weight > 0)

You should be aware though that this so-called "offline" version of PACTA has basically been abandoned and not maintained for many months now, so at some point its results may diverge from those that the web tool scripts produce.

I think this issue really needs to be discussed again because, as I have mentioned in the past, MFM relies on using the offline code (@looreen @Antoine-Lalechere ). But I can understand the reason for this, maintaining to different codes takes time, so do you see a possibility to run the web tool script offline to create similar results in the structure of the offline code? Or do you have a different idea to ensure consistency? I mean, for our credibility, PACTA results should not be different if an asset manager uses TM or sees his fund displayed on MFM.

superseded by #533

I think this issue really needs to be discussed again because, as I have mentioned in the past, MFM relies on using the offline code (@looreen @Antoine-Lalechere ). But I can understand the reason for this, maintaining to different codes takes time, so do you see a possibility to run the web tool script offline to create similar results in the structure of the offline code? Or do you have a different idea to ensure consistency? I mean, for our credibility, PACTA results should not be different if an asset manager uses TM or sees his fund displayed on MFM.

I don't know precisely what you mean by "similar results", but the web tool scripts do produce PACTA results very much like the other version. The primary difference is that it's increasingly trying to do just PACTA well, but not other things that complicate the process, so for instance, if one wants to run multiple portfolios, than the expectation is that they run each portfolio through the PACTA process individually. That might sound daunting or inconvenient, but I believe that looping through the PACTA process multiple times is something that should be done outside of PACTA, not inside. I'd be happy to do a tutorial session with you to get you up to speed on that.

I've personally walked through using the web tool scripts locally with @Antoine-Lalechere and I believe he is comfortable doing that on his own now, so maybe @Antoine-Lalechere can say whether that's adequate for MFM purposes since he has context from both sides?

The problem of the web_tool_scripts is that it can only run for 1 portfolio at a time and give output file by portfolio, which is not something that can be used as such by MFM as we use to run 12000 different portfolios. I've talked with @AlexAxthelm who said that a workflow suiting MFM needs (basically dividing by portfolio, then executing just PACTA on a single portfolio, then binding results needed by MFM (40_Results folder output) can be provided by PACTA team. @FrederickFa @looreen @cjyetman

Thanks @Antoine-Lalechere. That sounds like it will end up being something very similar to the "meta report data creator" you've used recently. Totally feasible, but does not yet exist.

this requirement (running Pacta offline for 12k portfolios) is also something that would be a big step towards a workflow suitable for supervisors. data sharing is still another issue then of course, but in principle, this sounds like many non-platform projects could benefit