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

Attached workflow breaks in GUI2 #9198

Closed
AdRiley opened this issue Feb 27, 2024 · 12 comments
Closed

Attached workflow breaks in GUI2 #9198

AdRiley opened this issue Feb 27, 2024 · 12 comments
Assignees
Labels
--bug Type: bug -gui p-highest Should be completed ASAP
Milestone

Comments

@AdRiley
Copy link
Member

AdRiley commented Feb 27, 2024

Reported by @jdunkerley

The attached workflow does not work correctly in GUI2.

ESG_Employee_Figures.zip

@jdunkerley will be able to provide the data files.

@AdRiley AdRiley added p-highest Should be completed ASAP --bug Type: bug triage -gui labels Feb 27, 2024
@AdRiley AdRiley added this to the Beta Release milestone Feb 27, 2024
@farmaazon farmaazon moved this from ❓New to 📤 Backlog in Issues Board Feb 27, 2024
@farmaazon farmaazon moved this from 📤 Backlog to ❓New in Issues Board Feb 27, 2024
@jdunkerley
Copy link
Member

Here is the rough summary:

There is an error in one of the grouped functions (build_energy_source).

When trying to repair this, the GUI won't evaluate (possibly due to the order of nodes being incorrect).
If you try and fix with the code editor (e.g. sorting the imports at the top of the file or manually fixing the function) then the file is not correctly saved.

In the end, once we deleted the metadata from the file we were able to repair it (though was still painful).

Editing any collapsed function also appears to cause a complete recalculation which likewise makes the process very painful.

With the metadata deleted, the main function was laid out correctly but grouped functions were laid out going upwards not downwards.

@farmaazon farmaazon added the s-info-needed Status: more information needed from submitter label Feb 27, 2024
@farmaazon
Copy link
Contributor

Without data, I cannot follow those steps: I have Unauthorized error.

There is, however, another bug: When opening this project and showing code editor, it seems to be constantly re-evaluated without my action (the errors flicker).

@jdunkerley
Copy link
Member

Data files used are:
gridData_summary.csv
Commuting.xlsx
BuildingUsage.csv
buildingEnergyStats.zip

For the postgres access I will message you separately

@farmaazon
Copy link
Contributor

Thanks, @jdunkerley. Yes, the project is very unstable. Actually, single entering node + trying to fix paths actually freezes IDE.

Investigation in progress.

@farmaazon farmaazon removed triage s-info-needed Status: more information needed from submitter labels Feb 27, 2024
@sylwiabr sylwiabr moved this from ❓New to 📤 Backlog in Issues Board Feb 27, 2024
@farmaazon farmaazon self-assigned this Feb 27, 2024
@farmaazon farmaazon moved this from 📤 Backlog to 🔧 Implementation in Issues Board Feb 27, 2024
@enso-bot
Copy link

enso-bot bot commented Feb 27, 2024

Adam Obuchowicz reports a new STANDUP for today (2024-02-27):

Progress: Took another issue with highest priority. investigated and fixed one problem discovered in James's project, then found the function which freezes our ydocs server during some edit. It should be finished by 2024-03-04.

Next Day: Next day I will be working on the same task. Fix the freeze problem and check another issues.

@farmaazon
Copy link
Contributor

farmaazon commented Feb 28, 2024

I know what is the problem: its combination of several factors:

  1. this project's 100 lines of code produced 240KB of idmap
  2. After a single edit, the idmap seems completely shuffled. The expected diff is large
  3. We try to apply O(ND) algorithm and are choked.

Not sure if there is a workaround. We could use the stupid O(1) algorithm from the old GUI... but in that case we likely send 240KB of edit to the engine on every edit, what is far from ideal.

The better solution would be to reduce size of idmap, (already reported as an issue) - however, we must be very efficient. Currently, a single entry takes 82-86 bytes; even if we reduce it to 12 bytes (two words for span boundary - may not suffice, and 8 bytes for UUID) we quickly get back to ~250KB when the modules will grow - and even a module of 1000 lines should be considered reasonable.

There is also an idea of getting rid of IdMap from file entirely, and make separate API for sharing Ids of AST nodes. In this API we could make a smarter diffing.

@farmaazon
Copy link
Contributor

farmaazon commented Feb 28, 2024

@jdunkerley you may check package with fixes: https://github.com/enso-org/enso/actions/runs/8079248374?pr=9207 I didn't check yet the problem of nodes ordering after removing metadata.

Also, I saw many duplicated imports - were they added by GUI?

GitHub
Hybrid visual and textual functional programming. Contribute to enso-org/enso development by creating an account on GitHub.

@jdunkerley
Copy link
Member

Definitely seems a lot better, thanks @farmaazon

@jdunkerley
Copy link
Member

Also, I saw many duplicated imports - were they added by GUI?

Yes still an issue.

mergify bot pushed a commit that referenced this issue Feb 29, 2024
Fixes probably #9198 See [this comment](#9198 (comment)) for justification. TLDR: the diff algorithm is too slow for our huge idmap.

The proper fix would be to reduce idmap size. Expect tasks for that soon.
@enso-bot
Copy link

enso-bot bot commented Feb 29, 2024

Adam Obuchowicz reports a new STANDUP for yesterday (2024-02-28):

Progress: Found the core issue of the freeze and implemeted a workaround. Progressed a bit with drop-down filtering. It should be finished by 2024-03-04.

Next Day: Next day I will be working on the same task. Apply review; I was asked to check more closely what threshold we should pick after which diff take too much time.

@enso-bot
Copy link

enso-bot bot commented Feb 29, 2024

Adam Obuchowicz reports a new STANDUP for today (2024-02-29):

Progress: Add benchmarks showing diff/fast diff efficiency on our metadata. Checked new tasks in backlog, made triage of some bug reports. Also bookclub and reviews. It should be finished by 2024-03-04.

Next Day: Next day I will be working on the #8942 task. I need to look at transient failures again. If time allows, progress more with widget input filtering.

@farmaazon
Copy link
Contributor

Made follow-up issues: #9255 and #9257.

I cannot reproduce the problem with imports. Let's see if it will happen in future projects.

@github-project-automation github-project-automation bot moved this from 🔧 Implementation to 🟢 Accepted in Issues Board Mar 4, 2024
@farmaazon farmaazon moved this from 🟢 Accepted to 🗄️ Archived in Issues Board Mar 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
--bug Type: bug -gui p-highest Should be completed ASAP
Projects
Archived in project
Development

No branches or pull requests

3 participants