Possible bug in Agent around spend_limit

[alexdewar]

I've been looking at the Agent class for reasons and noticed that it takes kwargs as an input to __init__(), but only uses this to check if it has a "spend_limit" element:

        if kwargs is not None:
            self.spend_limit = kwargs.get("spend_limit", 0)

1. This if-statement is broken. kwargs will never be None; if the user doesn't pass any extra arguments it'll be an empty dict instead. It's also not obvious what the intent with it was, because surely you always want to have a default value for spend_limit regardless?

2. ...which brings me onto my second concern. spend_limit appears to be being used without checking whether or not it has this sentinel value of 0 (this should probably be None), in which case presumably all non-free technologies will be excluded as being above the limit. I found one place where it is used (in filters.py: https://github.com/EnergySystemsModellingLab/MUSE_OS/blob/develop/src/muse/filters.py#L357). I'm guessing that either this functionality is never used or is broken? It's entirely possible that I've misunderstood the code though....

3. I figured we could probably drop the kwargs and just add spend_limit as an input argument. The problem is that if we do that, then we get a bunch of test failures for tests which seemingly rely on this behaviour of accepting arbitrary keyword args (in particular they seem to often pass a technologies argument; there may be others)... but why?! Optimistically we could just remove these unnecessary arguments and it'll just work.

Thougts @dalonsoa @tsmbland ?

[dalonsoa]

You're right about 1.

I'm not sure about 2 - will need to look into the detail of the code.

About 3, a lot of MUSE (the whole of it?) was designed in such a way that users will want and be able to expand its functionality (?). This includes creating new types of agents doing stuff differently and accepting different inputs. As far as I know, this has never happened, but the functionality is still there. I would not remove the kwargs altogether but simply make the sepend_limit explicit in the constructor defaulting to some value (0 or something else), and keep kwords there just to catch any argument that is not explicit.

[alexdewar]

Okey doke. I'll do that