Skip to content

Conversation

@Tyler-lc
Copy link
Contributor

@Tyler-lc Tyler-lc commented Jul 22, 2025

Short explanation

Corrected capitalization in auxiliary_settings.gms, that prevented output files from being compressed in UNIX systems.

Longer explanation

On case-sensitive file system (MacOS and Linux), GAMS is not compressing the output files as expected. This results in output gdx files that can be 3-4 times larger than on machines running Windows (which does not have a case sensitive file system).

The problem seems to be in a capitalization in the file "auxiliary_settings.gms". Currently we have:
"$SETENV GdxCompress 1"

However, it should be in all-caps:
"$SETENV GDXCOMPRESS 1"
The typo causes the command to be ignored on MacOS and Linux.

How to review

  • Possibly test if new capitalization works on Windows as well.

PR checklist

  • Continuous integration checks all ✅
  • Add or expand tests; coverage checks both ✅
  • Add, expand, or update documentation.
  • Update release notes.

@CLAassistant
Copy link

CLAassistant commented Jul 22, 2025

CLA assistant check
All committers have signed the CLA.

@codecov
Copy link

codecov bot commented Jul 22, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 96.2%. Comparing base (3742426) to head (c7bf2ec).
⚠️ Report is 3 commits behind head on main.

Additional details and impacted files
@@          Coverage Diff          @@
##            main    #965   +/-   ##
=====================================
  Coverage   96.2%   96.2%           
=====================================
  Files         60      60           
  Lines       5132    5132           
=====================================
  Hits        4940    4940           
  Misses       192     192           

@khaeru
Copy link
Member

khaeru commented Jul 22, 2025

@Tyler-lc thanks for the initiative in investigating, filing #964, and opening this PR. Please request a review when you've finished with the checklist.

One minor suggestion for now: per our code style, first bullet → The seven rules of a great Git commit messageitem 5, the commit message could be:

Correct capitalization of 'GDXCOMPRESS' in aux file

or even:

Capitalize 'GDXCOMPRESS' in MESSAGE settings

instead of:

Capitalized [indicative/past tense] …

git commit --amend or similar can be used to modify the commit message.

@Tyler-lc Tyler-lc self-assigned this Jul 22, 2025
@Tyler-lc Tyler-lc marked this pull request as draft July 22, 2025 12:34
@Tyler-lc Tyler-lc changed the title Fix: Corrected capitalization to GDXCOMPRESS in aux file Fix: Correct capitalization to GDXCOMPRESS in aux file Aug 25, 2025
@Tyler-lc
Copy link
Contributor Author

Hi @khaeru, I think this PR doesn't really require a change in the documentation, but only the "update release note". I have checked on message-ix documentation "how to contribute", but I'm still unsure how to update the release note.

@khaeru
Copy link
Member

khaeru commented Aug 25, 2025

this PR doesn't really require a change in the documentation

Agreed! That's usually the case for bug fixes: the documentation already describes what the code should do; the bug is that the code is not doing that. So by fixing the bug, the existing docs again become a true statement of the behaviour.

I'm still unsure how to update the release note.

Thanks for asking, and sorry the docs are not clear on that.

The file to be modified is RELEASE_NOTES.rst, here. You can click "🕑 History" in the top right to see the commit history for that file, which includes commits like 3e366ef. Try to make a similar commit with a similar message.

Some tips are:

  • Put the entry in the section for "Next release" and subsection for "All changes".
  • Use the same kind of grammar and wrapping as existing entries.
  • Follow the existing style for linking to this PR at the end of the first sentence.
  • Check that the entry looks OK in the built docs. 2 ways to do this:
    • Push to this PR branch and wait for ReadTheDocs to build the docs.
    • Follow the instructions to run make html in your local doc/ directory.

@Tyler-lc Tyler-lc marked this pull request as ready for review August 26, 2025 07:57
Copy link
Member

@glatterf42 glatterf42 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for this fix :)
I rebased the branch on main and adapted the second commit message so that it starts capitalized. Now looks good to me 🚀

@Tyler-lc Tyler-lc merged commit 8f55fcf into main Aug 26, 2025
44 of 48 checks passed
@Tyler-lc Tyler-lc deleted the issue/964 branch August 26, 2025 15:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants