Skip to content

Phi indicator#222

Draft
pStefanec wants to merge 1 commit intoindustrial-optimization-group:masterfrom
pStefanec:PHI_indicator
Draft

Phi indicator#222
pStefanec wants to merge 1 commit intoindustrial-optimization-group:masterfrom
pStefanec:PHI_indicator

Conversation

@pStefanec
Copy link
Contributor

@pStefanec pStefanec commented Mar 30, 2025

Migrating PHI indicator from desdeo-tools.

PHI calculation maybe works (at least it does not fail and provides seemingly reasonable values).

PHIDecision fails because the arrays in the assessment calculation have different shapes... Did it work in the desdeo-tools?
Also why in get_weights the main_rp param is not even used?

@pStefanec pStefanec requested a review from gialmisi March 30, 2025 15:58
@pStefanec pStefanec self-assigned this Mar 30, 2025
@gialmisi
Copy link
Contributor

Since I am not familiar with the phi indicator, could @light-weaver or @giomara-larraga have a look at the code? Assuming you are more familiar with the indicator. If not, let me know as well.

@gialmisi gialmisi requested review from giomara-larraga and light-weaver and removed request for giomara-larraga May 21, 2025 08:03
@gialmisi
Copy link
Contributor

@light-weaver will be checking this.

@pStefanec pStefanec changed the base branch from desdeo2 to master May 26, 2025 08:59
@pStefanec
Copy link
Contributor Author

@light-weaver could you take a look at the Phi indicator? On the last meeting you mention I should remove the classes. Do you want me to keep the PhiDecision functionality or it's not necessary (it was in desdeo1). I will have time to work on this until tomorrow evening

@light-weaver
Copy link
Member

@pStefanec. Treat PHI and PHIdecision as two different indicators. Have separate methods/functions to calculate them. Make sure that the function signatures are similar to the other methods in the file. Make sure that the assumptions stated at the top of the file are followed as well.

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

Successfully merging this pull request may close these issues.

3 participants