iiasa / message-ix-models

Tools for the MESSAGEix-GLOBIOM family of models
https://docs.messageix.org/models
Apache License 2.0
16 stars 31 forks source link

Too similar file names on macos #147

Open glatterf42 opened 5 months ago

glatterf42 commented 5 months ago

As pointed out by @measrainsey last year and now by @ravitby, this repository currently contains both demand.csv.gz and DEMAND.csv.gz in data/test/MESSAGEix-GLOBIOM_1.1_R11_no-policy_baseline. This seems to cause git conflicts on macos systems as they don't distinguish between capitalized and non-capitalized file names.

I'm not immediately sure how we can remedy this, possibly by renaming a file, but I'm not sure if the exact names are somehow important (e.g., correspond to some model parameter and need to be exactly the way they are for the file to be found).

ravitby commented 5 months ago

The result of the duplicate files is that there is always a change in the repo and therefor it's impossible to pull new changes. This is the output message for cloning the repo:

remote: Counting objects: 100% (3728/3728), done.
remote: Compressing objects: 100% (1448/1448), done.
remote: Total 25083 (delta 2505), reused 2374 (delta 2278), pack-reused 21355
Receiving objects: 100% (25083/25083), 19.41 MiB | 11.41 MiB/s, done.
Resolving deltas: 100% (15969/15969), done.
Updating files: 100% (565/565), done.
Filtering content: 100% (353/353), 89.53 MiB | 5.25 MiB/s, done.
warning: the following paths have collided (e.g. case-sensitive paths
on a case-insensitive filesystem) and only one from the same
colliding group is in the working tree:

  'message_ix_models/data/test/MESSAGEix-GLOBIOM_1.1_R11_no-policy_baseline/DEMAND.csv.gz'
  'message_ix_models/data/test/MESSAGEix-GLOBIOM_1.1_R11_no-policy_baseline/demand.csv.gz'
glatterf42 commented 5 months ago

Thanks for the additional input. Searching for this error message, I came across this suggestion to set up a new, case-sensitive volume on macos that can host the existing code. However, I'd consider it more elegant if we could avoid using the same name for these files. I hope @khaeru knows if such an adaptation is straightforward.

glatterf42 commented 5 months ago

There's also a git config variable that we could play around with to see if some setting satisfies our needs locally.

khaeru commented 5 months ago

This is an unfortunate consequence of the fact that the MESSAGE GAMS code uses both a parameter named demand and a variable named DEMAND. See: https://github.com/iiasa/message_ix/blob/693f2273eaf31aec7a9938f65ee26f98be967742/message_ix/model/MESSAGE/parameter_def.gms#L147-L159 https://github.com/iiasa/message_ix/blob/693f2273eaf31aec7a9938f65ee26f98be967742/message_ix/model/MESSAGE/data_load.gms#L75-L76 https://github.com/iiasa/message_ix/blob/693f2273eaf31aec7a9938f65ee26f98be967742/message_ix/model/MESSAGE/model_core.gms#L149-L151

The model data snapshot published on Zenodo contains data for both, and so the 'exploded' or 'unpacked' version of that file stored in this repo has both.

AFAIK, the variable DEMAND (upper case) is not relied on by any tests, currently, so I think as a mitigation the corresponding file could be safely deleted. If/when we add or migrate (from message_data) code that relies on data in this variable, we would need to find a different approach.

measrainsey commented 5 months ago

Hi all, thanks for addressing this. I think I'm the source of the issue because I'm a macOS user 🥲 Like @glatterf42 said, my computer does not allow me to have files with the same spelling/name (even if they have different capitalizations). I've tried to deal with this a few times while working on my PR in message-ix-models, such as when I tried to add back the DEMAND file (in https://github.com/iiasa/message-ix-models/pull/99/commits/b761a93cf30bb0a86383a2238a29f091948cf3fb), only to later have to delete it again (in https://github.com/iiasa/message-ix-models/pull/99/commits/23ae6ffa9813f7832bb797251b6f150e0a556e66). So currently no real fix on my end.

glatterf42 commented 5 months ago

I think we can leave this open since the merged mitigation PR is likely just a temporary fix; however, for the time being, this issue should be solved on main :)