Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Order of Timeslices is critical for TIMES, but not respected by xl2times #270

Open
Antti-L opened this issue Dec 30, 2024 · 6 comments · Fixed by #272
Open

Order of Timeslices is critical for TIMES, but not respected by xl2times #270

Antti-L opened this issue Dec 30, 2024 · 6 comments · Fixed by #272
Labels
bug Something isn't working

Comments

@Antti-L
Copy link

Antti-L commented Dec 30, 2024

While I have just made my first simple tests with xl2times, out of curiosity I also tested whether xl2times respects the Timeslice order, which is a fundamental basic requirement, and critical for correct model behaviour. Timeslices are an ordered set, which is an important feature of GAMS. So, for example, if I define the SEASON timeslices to represent months, I should be able to define them simply as follows in Excel:

image

I just tested that, but to my surprise and dismay, it did not work! Xl2times wrote the timeslices out in the alphabetic order instead of the correct monthly sequence. In my view, this is a critical oversight, which I guess cannot even be seen with gdxdiff. Arranging the monthly timeslices in an alphabetic order does not make sense, and because of that, GAMS will neither get the correct order. VEDA does respect the order, as defined by the user, and so should xl2times.

@Antti-L Antti-L added the bug Something isn't working label Dec 30, 2024
@Antti-L Antti-L changed the title Order of Timelices is critical for TIMES, but not yet respected by xl2times Order of Timeslices is critical for TIMES, but not yet respected by xl2times Dec 30, 2024
@olejandro
Copy link
Member

Thanks @Antti-L!

@siddharth-krishna what kind of a test could we do for this? I am almost 100% sure that at least at some point the order was preserved, but since we don't test for this, it must have got broken with other improvements.

@olejandro
Copy link
Member

@Antti-L I've opened a PR preventing ALL_TS from being sorted when writing out ts.dd. Are there any other sets for which this is relevant?

@Antti-L
Copy link
Author

Antti-L commented Jan 4, 2025

I've opened a PR preventing ALL_TS from being sorted when writing out ts.dd.

That's great.

Are there any other sets for which this is relevant?

I don't think so. As far as I know, all other ordered sets in TIMES are system sets, and not user-defined.

@siddharth-krishna
Copy link
Collaborator

Thanks, Olex, for the quick fix.

You're right Antti, gdxdiff doesn't seem to recognize the ordering as a difference when comparing our output to the ground truth. I notice that the ground truth also defines the set in the DD file as SET ALL_TS, is there some GAMS keyword to indicate that the set is ordered? Or are all sets ordered by default, and it is just that TIMES does not depend on the ordering of other sets?

If you don't mind, I'll keep this issue open to remind myself to make a test for this. One option could be for us to modify the comparison code similar to Olex's fix in #272, so that it doesn't sort ALL_TS before comparing.

@Antti-L
Copy link
Author

Antti-L commented Jan 6, 2025

I notice that the ground truth also defines the set in the DD file as SET ALL_TS, is there some GAMS keyword to indicate that the set is ordered?

No, there is no GAMS keyword to indicate that the set is ordered. See: https://www.gams.com/latest/docs/UG_OrderedSets.html

Or are all sets ordered by default, and it is just that TIMES does not depend on the ordering of other sets?

Not really by default. In general, sets in GAMS are regarded as unordered collections of labels. However, as described in the GAMS documentation, all set elements of all sets are included in the so-called universal set, in the order of data entry. And only if the elements of a set are defined in an order consistent with that entry order, the set is considered to be ordered. Example:

  • SET AA / 7, 9, 3, 2 /;
  • SET BB / 1, 2, 3, 4, 5, 6, 7, 8, 9 10 /;

The set AA will be ordered, because it is the first set defined, and therefore the order of the elements are always the same as the entry order of them in the universal set. However, the set BB will not be ordered, because the order of the elements, as defined, is inconsistent with the entry order in the universal set. This is important, because the lead and lag operators can be used only with ordered sets, and any looping over sets in the defined order (without creating extra index sets) can be correctly done only with ordered sets (or their underlying ordered domains). The set AA would remain ordered even if one would subsequently add some more elements into it, as long as the order remains consistent, e.g. by adding additional elements of BB into it as follows:

  • SET AA / 1, 4, 5, 6, 8, 10 /;

But yes, what I mentioned above about all other ordered sets (apart from ALL_TS) in TIMES being system sets, meant that I was referring to those sets that are required to be ordered in TIMES. As described above, other user-defined sets could also in fact be ordered, depending on how the user defines them, but except for ALL_TS (and MILESTONYR), TIMES does not depend on any other user-defined set to be ordered. For ALL_TS the ordered nature is necessary for the handling of timeslices correctly, and therefore it should best be the first set defined in TIMES (as done in the RUN files). If the user nonetheless accidentally violates the TIMES set ordering requirements, GAMS will issue an error.

@olejandro olejandro changed the title Order of Timeslices is critical for TIMES, but not yet respected by xl2times Order of Timeslices is critical for TIMES, but not respected by xl2times Jan 6, 2025
@Antti-L
Copy link
Author

Antti-L commented Jan 9, 2025

Now that I recalled it, I added MILESTONYR above as another user-defined set that is required to be ordered. But in the case of MILESTONYR the situation is much easier: GAMS will immediately issue a compile-time error if the order of MILESTONYR is incorrect (because the underlying ALLYEAR domain is a system set). So, ALL_TS is basically the only critical case.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants