gpitt71/gemact-code

TypeError when using LayerTower with at least 2 items with default share

Closed this issue · 4 comments

C. f. the minimal example appended. Trying to make it running, I discovered several issues.

At least four issues arise:

  1. DEDUCTIBLE_TYPE is not defined in config.py. I have tried to edit the corresponding code file in Github, but somehow my changes were lost. (Perhaps, I do not have the right to edit this file.)
    Therefore, I propose to add the line:
    DEDUCTIBLE_TYPE = {None, 'non-ranking eel', 'eel'}
    as new line 10 to config.py. (Assuming that "None" is not an allowed value, i. e. that the default Nones are always converted to correct values before checking.)
  2. The bound checks for the upper bound are not correct, perhaps due to a copy&paste error. The correct version in helperfunctions.assert_type_value() should be:
                message = 'Make sure %s is lower than or equal to %r.' % (name, upper_bound)
                assert value <= upper_bound, logger.error(message)
            except AssertionError as msg:
                print(msg)
        else:
            try:
                message = 'Make sure %s is lower than %r.' % (name, upper_bound)
                assert value < upper_bound, logger.error(message)

Note that there are 4 substitutions of lower_bound by upperbound. (within the message text too.)
3. When this was corrected, somehow the PolicyStructure share value [1.0] was compared against its bounds, raising a type error. This can be fixed by deleting the square brackets in line 76 in LossModel._share_and_layer_check(), i. e. by changing the line to
self.shares = self.shares * len(self.layers)

Note that I tacitly assume here that the list self.shares shall have no dictionary entries, but repeated values. Is this assumption correct? (For two layers without share, this would mean [1.0, 1.0] instead of [[1.0], [1.0]] )
4. Finally, PolicyStructure seems to work but LossModel raised an error:

Traceback (most recent call last):
  File "C:\Users\xy\Desktop\Minimal Working Example.py", line 14, in <module>
    modell = gem.LossModel(frequency=gem.Frequency(
  File "C:\Users\xy\AppData\Roaming\Python\Python311\site-packages\gemact\lossmodel.py", line 383, in __init__
    self.dist = dist
  File "C:\Users\xy\AppData\Roaming\Python\Python311\site-packages\gemact\lossmodel.py", line 393, in dist
    hf.assert_member(value, config.DIST_DICT.keys(), logger, config.SITE_LINK)
  File "C:\Users\xy\AppData\Roaming\Python\Python311\site-packages\gemact\helperfunctions.py", line 297, in assert_member
    raise TypeError('choice must be a ``list``, ``set`` or ``dict``')
TypeError: choice must be a ``list``, ``set`` or ``dict``
  1. A question of design: Currently, an error occurs, is printed as a message without much context and the program occurs. Is this always intended or should e. g. unfulfilled checks re-raise the error with an additional explaining message? (c.f. https://www.geeksforgeeks.org/python-reraise-the-last-exception-and-issue-warning/)

Thank you very much for your feedback,

I am sure that your errors arise because you are currently working with a version that we are completing locally.
In a few days we will upload the new version of the software and the errors you displayed should be solved.

I will leave this issue open and come back to you.

Wonderful, thanks! But please enjoy Christmas and New Year without seeing yourself forced to program. :-)

EdoLu commented

Dear @Friedrich2 , the issues you pointed out should be solved now. Let us know in case there is anything we might have missed. Please note that we haven't versioned the package in PyPI yet. We will do it in the next few days.

Well done, thanks! The issues I found are indeed solved.