modelica / ModelicaStandardLibrary

Free (standard conforming) library to model mechanical (1D/3D), electrical (analog, digital, machines), magnetic, thermal, fluid, control systems and hierarchical state machines. Also numerical functions and functions for strings, files and streams are included.
https://doc.modelica.org
BSD 3-Clause "New" or "Revised" License
471 stars 168 forks source link

Impure function called from pure function MemoryBase.getMemory #3855

Open MarkusOlssonModelon opened 3 years ago

MarkusOlssonModelon commented 3 years ago

I have found an issue in Modelica.Electrical.Digital.Interfaces.MemoryBase.getMemory. It contains a function call to Modelica.Utilities.Streams.readLine which is an impure function, but getMemory is not declared as impure.

I suggest adding the impure prefix to getMemory. This will also require changes in Modelica.Electrical.Digital.Memories.DLATRAM and Modelica.Electrical.Digital.Memories.DLATROM since they contain calls to getMemory. For that I think the simplest solution is to change the if-statements surrounding the calls to when-statements.

HansOlsson commented 3 years ago

The functiongetMemoryshould not be declared as impure, but it should be handled in another way. Basically the function getMemory is intended to be behave in a pure way; and it is primarily used with tables given in the library.

HansOlsson commented 1 year ago

Looking at it once more I think that purity is the least of our concerns.

Superficially it seems to do the following:

Fill a logical table with binary values, 0, 1, 2, ... 2^(n_addr)-1, with n_data bits from left-to-right, stored as L.'0', L.'1' (and L.'X' for the left-over bits).

For some reason that is done using an external table which makes it impure, and fails to handle n_addr>4 and if n_data>4 the rest of the bits are left-over bits.

So, if we don't change the external table, I would have the following questions:

If the intent is to provide a different external table the obvious improvement would be to have a table in Modelica instead, so that you don't have to set n_addr, n_data, and external table separately.

And note that this is just how the memory is initialized (the RAM-model allows writing as well). Assuming that the models are useful I would believe it would make more sense to have a selectable memory-initialization-function; like generalization of current one, fill with some value (it seems the original model just filled with 'U' - as the start-value), or user-selectable.