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

FIX: Sneaky bugs reported by Patrick McGuire #102

Merged
merged 2 commits into from
Aug 28, 2024
Merged

FIX: Sneaky bugs reported by Patrick McGuire #102

merged 2 commits into from
Aug 28, 2024

Conversation

nocollier
Copy link
Collaborator

No description provided.

@nocollier nocollier merged commit 0a26232 into master Aug 28, 2024
2 checks passed
@nocollier nocollier deleted the mcguire branch August 28, 2024 18:22
@mcguirepatr
Copy link

Hi Nathan:
Thanks for trying to fix this.
It modifies the behavior, but not exactly how I expected.
The overall scores for the relationships are now missing for all the models, whereas I would only expect them to be missing for the relationships for the models that have missing data.
See this link:
https://gws-access.jasmin.ac.uk/public/ncas_climate/pmcguire/ILAMB_output/GCP_290824a/
Or this screenshot:
Screenshot 2024-08-29 at 14 07 27

I didn't rerun the whole ILAMB, I just reran ILAMB with these two lines in the Confrontation changed and I added a new model (SDGVMv9_24) prior to the rerun.
Can you take a 2nd look at it?
Thanks
Patrick McGuire

@nocollier
Copy link
Collaborator Author

nocollier commented Aug 29, 2024 via email

@minxu74
Copy link
Contributor

minxu74 commented Aug 29, 2024

@nocollier I tried to look into the problem by open the UD (https://gws-access.jasmin.ac.uk/public/ncas_climate/pmcguire/ILAMB_output/GCP_290824a/dashboard.html). I found the results from UD and index.html were different, but they are supposed to be identical. The reason is that the contents of scalars.json are different to scalar_database.json used by UD. The same problem also reported by #100 .

@nocollier
Copy link
Collaborator Author

This is a problem of me having bolted on things to ILAMB. Thanks Min, this gives me a place to start looking. I think the issue is that there are multiple places where we merge results. There is the OverallScore function of Confrontations, but then I also (later) wrote a harvest of all scalars and compute aggregations using pandas. I will track them all down and let's see if we can unify them so it is a single way.

@minxu74
Copy link
Contributor

minxu74 commented Aug 29, 2024

@nocollier Thanks a lot. I found the code and could fix the inconsistent problem if you are not available. But we need to discuss a way to compute composite scores and write the two json files consistently. We also could let index.html read the CMEC json file directly as the CMEC json file includes all the results and meta information.

@nocollier
Copy link
Collaborator Author

I believe the issue is that we need aggregation in 2 places:

  1. Before generating the Dataset html pages. The score data is written into the html directly and so we need it at this level.
  2. When leaving an artifact for use in the top level page.

I finished my reviews, will look at this today/tomorrow.

@mcguirepatr
Copy link

I ran out of space on my web server, so I am currently moving my ILAMB pages to another web server. (Just in case you're looking for any of them.)

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.

3 participants