JGF simplification #1293
Merged
JGF simplification #1293
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
jameshcorbett
added a commit
to jameshcorbett/flux-coral2
that referenced
this pull request
Sep 5, 2024
Problem: JGF is too large, as described in flux-framework#193. As a first step to shrink it, do not output default values, instead skipping them and letting the JGF reader supply them. Depends on changes introduced in flux-sched by flux-framework/flux-sched/pull/1293.
jameshcorbett
force-pushed
the
jgf-simplification
branch
4 times, most recently
from
6efc769 to
3f7adc8
Compare
jameshcorbett
added a commit
to jameshcorbett/flux-coral2
that referenced
this pull request
Sep 18, 2024
Problem: JGF is too large, as described in flux-framework#193. As a first step to shrink it, do not output default values, instead skipping them and letting the JGF reader supply them. Depends on changes introduced in flux-sched by flux-framework/flux-sched/pull/1293.
jameshcorbett
force-pushed
the
jgf-simplification
branch
3 times, most recently
from
1a640ad to
4e92c1e
Compare
|
OK, tests are failing, but I think it's generally working properly--the flux-coral2 testsuite runs successfully with the JGF generated from this branch. So remaining work is eliminating the |
Open
jameshcorbett
force-pushed
the
jgf-simplification
branch
3 times, most recently
from
250a48c to
751621f
Compare
|
Ok, only |
jameshcorbett
force-pushed
the
jgf-simplification
branch
2 times, most recently
from
90628a4 to
4716478
Compare
2 tasks
trws
reviewed
Sep 30, 2024
trws
approved these changes
Sep 30, 2024
Problem: the JGF reader's vertex-unpacking logic is unnecessarily complicated. Simplify it.
Problem: as described in flux-framework#1255, JGF is too large and unwieldy, with lots of redundant and repeated data. To begin to cut back, make most vertex metadata optional.
Problem: the Python JGF writer is too verbose, and outputs a lot of redundant data which the jgf reader should be able to construct for itself. Change the writer to output only non-default entries.
Problem: the python JGF writer no longer outputs certain vertex metadata, but the tests expect it to be there. Change the tests to no longer look for the eliminated metadata.
Problem: the timeouts waiting for files to appear in t1018 are incredibly long. If the file is going to appear, it should appear within twenty seconds. Reduce the timeout from 10 minutes to 20 seconds.
Problem: the JGF writer supplies the ID of nodes based on the hostname suffix (assuming it exists), which determines which nodes are matched first under policies like high and low. However, the JGF reader could just as easily supply the ID itself, thereby reducing the size of JGF. Make the reader supply the ID instead of the writer.
jameshcorbett
force-pushed
the
jgf-simplification
branch
from
4716478 to
6785369
Compare
|
Thanks, rebased with master, setting MWP. |
jameshcorbett
added
the
merge-when-passing
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #1293 +/- ##
========================================
- Coverage 75.3% 75.3% -0.1%
========================================
Files 111 111
Lines 15262 15298 +36
========================================
+ Hits 11505 11530 +25
- Misses 3757 3768 +11
|
jameshcorbett
added a commit
to jameshcorbett/flux-coral2
that referenced
this pull request
Sep 30, 2024
Problem: JGF is too large, as described in flux-framework#193. As a first step to shrink it, do not output default values, instead skipping them and letting the JGF reader supply them. Depends on changes introduced in flux-sched by flux-framework/flux-sched/pull/1293.
jameshcorbett
added a commit
to jameshcorbett/flux-coral2
that referenced
this pull request
Oct 1, 2024
Problem: JGF is too large, as described in flux-framework#193. As a first step to shrink it, do not output default values, instead skipping them and letting the JGF reader supply them. Depends on changes introduced in flux-sched by flux-framework/flux-sched/pull/1293.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Posting this as a WIP so I can link to it / work against it in flux-coral2.