ReaderTMY3 does not work for data going over into the next year

[AnaConstantin]

Hello everyone,

I have my own weather data, which I have adapted to a TMY3 format, in order to use the ReaderTMY3. The data covers the period of June 2016 to October 2017. The time stamp I used in the TMY3 format sets 1.01.2016 00:00 to 0s. However I get an error at 01.01.2017 00:00 and the simulation stops. Is there a way around this, as depending on my application, I might have data stretching over more than a year anyway? Or am not setting some parameter correctly?
Otherwise I'm happy to assist in extending the ReaderTMY3 to deal with such a situation.

[mwetter]

The ReaderTMY3.mo file is simply reading a .mos file. The 2nd line of this file has the number of columns. Maybe you made a mistake in this entry?

[AnaConstantin]

@mwetter thank you for your suggestion. Unfortunatelly it was not that. The description of the table in the second line is correct, otherwise the simulation wouldn't start.
To my mind the problem is in the ConvertTime-Block (conTim). Once the time stamp jumps into the next year, the calendar time (calTime) used for the interpolation of the table values jumps from 31539600s to 0s and the interpolation of the table values produces values outside the set bounds which because of some assert statements lead to a break in the simulation. The way I understand it, the advantage of this component, is that it allows a repeat of the table with annual data, if the simulation should strech over one year, but it does not allow the reading of values in a table with a time stamp over 31539600s. If this is the case I might have an idea to solve it by using a Modelica.Blocks.Sources.CombiTimeTable.
However this is just speculation on my side. I'm happy to hear your thoughts. Thank you in advance for your answer.
I have attached a snippet of my weather table which exemplifies this problem. As GitHub does not support .mos format, I saved it in .txt format and has to be re-saved in .mos format to work. I have done the simulation using both Lsodar and Euler as solvers, using the newest version of the Modelica IBPSA Library and Dymola 2018.
ExampleFile_error.txt

[mwetter]

@AnaConstantin : You are right, the issue is caused by the ConvertTime block.
I made a validation case on branch issue842_multiyear but won't have time this week to implement a fix. I think what should be done is

1. Adding functions IBPSA.BoundaryConditions.WeatherData.BaseClasses that get the first and last time stamp of the weather file, and the time increment if it is constant.
2. If the weather file spans a whole year, keep the current implementation.
3. If the weather file does not span a whole year (minus one time increment, e.g., 8759 is the last entry for an hourly file), don't use the ConvertTime block, and write an assert if the data are insufficient long (taking into account the 1800 s time shift that is done for the solar radiation).

[AnaConstantin]

Thank your for your answer @mwetter. I'd be happy to give it a try and implement your suggestion.

[mwetter]

@AnaConstantin : It would be nice if you can make an implementation and issue a pull request against the issue842_multiyear branch.

[thorade]

@AnaConstantin have you started to work on this? Otherwise I could do it today or tomorrow.

[AnaConstantin]

@thorade I haven't started yet: vacation, a lot of work in the office and a bit of a dilemma regarding the solution. While points 2 & 3 from what @mwetter suggested are easy to implement, regarding point 1 I wanted to look into the C-Functions from ModelicaStandardTables.c to see if something cannot be reused, before writting new functions. But I woun't get around to it until next week so feel free to implement your solution.

[thorade]

@AnaConstantin If you have an idea how to fix it and have time next week, then I believe you should do it. I would just do it so it gets fixed.

[AnaConstantin]

Ok, I give myself a deadline by next wednesday.

[AnaConstantin]

Hi everybody. So I did the modifications, however I do not have permission to push anything in the issue branch. Should I do a work around with a fork and a pull request? (not a big fan, especially since the Wiki recommends the workflow over branches directly).
Annyway, quick summary of what I did:

1. Added a function to get the start and end time of the weather file. It's not as fancy as I wanted, because I didn't get a good idea on how to use the ModelicaStandardTables functions to this purpose.
2. In ReaderTMY3 I condition the connection to the convertTime blocks by the fact that the weather data must span a year or less, but do not go over 31.12 24:00. Otherwise it the time for the weather data is taken directly from the clock of the simulation.
   I am however not a fan of this ConvertTime block. If all it should do is repeat a year, it does not do it smoothly: if the simulation goes over 31.12 it does lead to spikes in data, when switching to a following year and repeating the first year.

[thorade]

Yes, you should fork the IBPSA repository, push to the branch in your fork, and submit a pull request from your issue branch to the issue branch with the same name here in the IBPSA repo.

[mwetter]

@AnaConstantin : I merged your code to the branch issue842_multiyear.
However, I don't like the current implementation as it does not work if timeSpan[1] = -1. Also, the conditional connects in

  // Evaluate time span of weather data
  if ((timeSpan[1] >= 0.0) and (timeSpan[2] <= 31536000.0)) then // Data spans one year or less and ends no later than 31.12 24:00
    connect(conTim1.calTim, datRea1.u) annotation (Line(
        points={{-89,190},{-58,190}},
        color={0,0,127},
        pattern=LinePattern.Dot));
    connect(conTim.calTim, datRea.u) annotation (Line(
      points={{-99,-30},{-68,-30}},
      color={0,0,127},
      pattern=LinePattern.Dot));
  else
    connect(add.y, datRea1.u) annotation (Line(
      points={{-119,190},{-120,190},{-120,152},{-58,152},{-58,190}},
      color={0,0,127},
      pattern=LinePattern.Dot));
    connect(modTim.y, datRea.u) annotation (Line(
      points={{-159,0},{-150,0},{-150,-68},{-68,-68},{-68,-30}},
      color={0,0,127},
      pattern=LinePattern.Dot));
  end if;

require a translator to generate C code, run the C code to get the values for the timeSpan, and then process the connect statement.
Looking at it, I think a cleaner implementation would be to revise ConvertTime so that it handles the cases of timespan in the weather file not being equal to one calendar year, e.g., timeSpan[1] can be
negative, 0 or positive, and the same for timeSpan[2], where timeSpan[2]-timeSpan[1] may be larger than 1 year. If timeSpan[2]-timeSpan[1] is not equal to a multiple of a year, then wrapping around is OK for 30 minutes (as this is needed because of the time shift in solar radiation) but afterwards an error should be triggered.

[AnaConstantin]

@mwetter, thank you for your comments. I'm on maternity leave right now so I'm somewhat slower to answer or implement anything.

1. I didn't think about negative time stamps, since for me they are quite abstract. Do you have a particular example in mind where they might happen, or is it the idea of being rigurous in the implementation and cover the whole spectrum of possible values?
2. Your remark about translating and running the C-code and then processing the connect statement is interesting. I learned something new. I agree that the ConvertTime block being the one which causes the problems should be revised. However if we do this I suggest that we also implement a smoother transition to the beginning of the weather file if exactly a year of values are provided (see my comments from 15.11). Do you have a prefered way of doing this?
   Let me know what you think.

[mwetter]

@AnaConstantin : I am glad to hear about your paternity leave.

A negative time stamp happens if a user simulates for example from t0=-86400 to t=86400. We should be rigorous in the implementation as otherwise, it won't work for some users, and not only cause problems for the user, but also takes us more time if we need to fix the code later again.

Regarding the discontinuity in the weather data between Dec. and Jan., I think this should be corrected by correcting the weather data file. In the model, we don't know what the final time is, and hence I don't think it would be possible to fix this without affecting the results that one would have if exactly a year (from Jan. 1 to Dec. 31) is simulated. As the discontinuity is at midnight, I don't think there are bad implications on the simulation.

[AnaConstantin]

@mwetter sorry for taking so long to come back to working on this issue. While on maternity leave taking care of small children is for me more fun than Modelica. But after receiving a friendly reminder I want to get the issue sorted out in the next couple of days.
I will do the implementation for the timeSpan[2] -timeSpan[1] to cover all possible cases and do it inside the ConvertTime block. I still think the discontinuity is a problem, as it might not necessarily happen at midnight, depending on what type of data the user has, but I'll try to put a warning in place.
The pull request #858 can be deleted as it refers to on older version, if this comes up in a clean-up of the pull issues.

[mwetter]

@AnaConstantin Thanks for the update. I deleted #858

[mlauster]

To-Do on branch https://github.com/ibpsa/modelica-ibpsa/tree/issue842_multiyear:

• Clean up examples
• Do another round of code review
• Create PR to master

[Mathadon]

@mlauster @mwetter the current implementation has

initial equation 
  tStart = integer(modTim/year)*year;
equation 
  calTim = modTim - tStart;

such that the input of the CombiTable1Ds is shifted by an integer number of years depending on the initial value of time. I don't see the point in doing this. E.g. if a weather data file is indexed by a unix time stamp (in the order of 1.5e9) and the simulation time is set to the first value of this weather data file, the weather data reader will still start reading at t=0?, which is not contained by the weather data file.

I guess we have to agree first on the intended functionality of the weather data reader. For me this is:

1. Read weather data from file using the Modelica built-in variable time as an index for the first column of the file. I think this is the most intuitive and easiest to understand.
2. Optional: shift time by a fixed value to compensate for a time zone mismatch between the weather file and simulation time.
3. Optional: when time is outside of the table range, a) throw an error or b) cycle back to the first year in the file.

I wouldn't add 2) since this parameter is too hard to interpret for the average user and for 3) I would add both options using an enumerate and default to throwing an error.

Also, I think we should clearly document somewhere what 'time' means: does this relate to the position of the sun, or does it relate to the calendar time (which is different in different positions on the globe). I always forget..

@mwetter @mlauster @AnaConstantin do you agree if I modify the code such that we have the options described in 3) and if I modify the code such that the shift is removed such that we get 1)?

[mlauster]

@Mathadon, I think @mwetter and @AnaConstantin opinions are of higher importance than mine. So lets wait for their feedback. I will in the meantime start to add tests and check the code.

[AnaConstantin]

Hello everyone, I'll be quick since it took a long time for me yesterday to go through everything which was done and said on this issue (and additional pull requests) already:
@mlauster there already are examples done and the code has been checked by TravisCI since I did all the development in a forked branch, which has been merged (see above). What is missing are reference results for the examples since I'm waiting for feedback from @mwettter on the implementation to see if I keep the examples as such.
@mwetter if I somehow missed your latest feedback on this issue, please let me know.
@Mathadon the point of the equations you mentioned are just to repeat a whole year (or a multiple of it). Regarding option 2), I think this is already taken into account in the current implementation as this information is extracted from the hearder of the TMY3 file. Regarding t=0, solar time and actual time, this is I think explained in the documentation of ReaderTMY3.mo, under "Implemenation".

[mwetter]

@Mathadon : I agree partially with using your option 1 "Read weather data from file using the Modelica built-in variable time as an index for the first column of the file." This is OK for all data except those read by datRea1 which must be shifted by 30 minutes as explained in the implementation notes of the weather data reader.

I agree that tStart = integer(modTim/year)*year; should be removed because it does not allow to start reading a weatherfile at tStart = 2 years.

It would be nice if we could use Modelica.Blocks.Sources.CombiTimeTable but this is not possible because it contains the lines

  timeScaled = time/timeScale;
  when {time >= pre(nextTimeEvent),initial()} then

which for the solar radiation data should be

  timeScaled = (time+1800)/timeScale;
  when {(time+1800) >= pre(nextTimeEvent),initial()} then

Maybe easiest would be to copy Modelica.Blocks.Sources.CombiTimeTable, add a parameter

parameter Modelica.SIunits.Time timeOffset 
  "Time offset, causing data to be read at time+timeOffset";

and delete ConvertTime.mo.

@AnaConstantin : The scripts IBPSA/Resources/Scripts/Dymola/BoundaryConditions/WeatherData/Validation/OverAYear_usingOneYearData.mos and IBPSA/Resources/Scripts/Dymola/BoundaryConditions/WeatherData/Validation/ThreeYears_usingTwoYearData.mos are missing.

Note that I merged the master into this development branch as we had 660 commits since then.

[AnaConstantin]

@mwetter I assume you mean the scripts for the unit tests. As discussed previously I was unable to get access to the software configuratuion needed for generating unit tests. Neither at the institute nor at home where I'm working from at the moment. I'm very sorry for this, but I could not do anything about it. Let me know if I can help in any other way.

[mwetter]

@mlauster : Can you please write and add such scripts that can be used to test variables of interest.

[mlauster]

@mwetter, that's fine, I will take overt this work. But give me some time, we are in the reporting season and I still need to take care of my BS Paper. :-)

[mwetter]

@AnaConstantin : I merged your pull request but need some more time for testing. The old implementation did not work if the start time of the simulation is negative, which I corrected in b788c32

Can you please also run some tests to make sure the model works for startTime being negative, zero and strictly positive.

[AnaConstantin]

Ok, I'm officially back to work tomorrow so I'll start with this.