Hive-Systems / pyfair

Factor Analysis of Information Risk (FAIR) model written in Python. Managed and maintained by Hive Systems
https://www.hivesystems.com
MIT License
89 stars 45 forks source link

Some input_data abbreviations are failing #27

Closed vidhisshah closed 4 years ago

vidhisshah commented 4 years ago

These inputs throw a key error:

model.input_data('C', low=10, mode=30, high=40)
model.input_data('CF', low=10, mode=30, high=40)
model.input_data('Contact Frequency', low=10, mode=30, high=40)

This works fine: model.input_data('Contact', low=10, mode=30, high=40 )

Similarly,'A', 'Probability of Action' fail but'Action'works.

I've tested all other abbreviations from target_map and they seem to work fine. Please do check.

theonaunheim commented 4 years ago

Thanks, @vidhisshah . Will check this evening and reply back.

theonaunheim commented 4 years ago

@vidhisshah ,

Thanks for your detailed report. Much appreciated.

FairModel has an attribute called _tree. This attribute is a FairDependencyTree instance that is used to track calculation dependencies. This class was using erroneously using "Contact" and "Action" as handles for tracking, which is where the issue arose.

I am going to take this fix and another I've been meaning to get out (#24) and package them up this weekend for version 0.1-alpha.11.

For posterity. going forward these will work:

These will not work:

theonaunheim commented 4 years ago

@vidhisshah ,

Would you please download version 0.1-alpha.11 from PyPI, ensure it fixes your problem, and then close this issue out?

Thanks again for the bug report.

vidhisshah commented 4 years ago

@theonaunheim Thanks a lot! The keywords 'C','Contact Frequency','A', and 'Probability of Action' are working fine.

One small issue though, it doesn't seem to be doing the check where values need to be between zero and one. Could you please check that? For example, model.input_data('Probability of Action', low=2, mode=10, high=20) would have thrown an error earlier, but it seems to be accepting it now.

theonaunheim commented 4 years ago

Brilliant, and thanks for your attention to detail, @vidhisshah !

It appears that FairDataInput validator and accompanying unit tests use the term "Action" as well. I will modify FairDataInput._le_1_targets attribute, look for any stray "Action" references in the code base, and check back with you in the next day.

theonaunheim commented 4 years ago

@vidhisshah , I believe I have fixed the validation problems.

At your convenience, would you please check version 1.0-alpha.12 on your machine, and close out this issue if fixed?

Thanks again for your diligence.

vidhisshah commented 4 years ago

Works! Thank you.