Comparison Tool 4.5: Grid of Pie Charts across Cooling Times by eitan-weinstein · Pull Request #179 · svalinn/ALARA

eitan-weinstein · 2025-12-02T00:52:51Z

Closes #174.

Expanding on pie chart functionality added in #145 to create a grid of pie charts for all cooling times for a given run and response variable. An example of what one of these grids looks like is attached:

gonuke

Nicely formatted and put together here... I have a number of suggestions to

help separate concerns a little and
take advantage of dataframes without creating so many new data structures

gonuke · 2025-12-02T12:57:07Z

tools/alara_output_processing/alara_output_plotting.py

+    # Build and populate table
+    table_ax = fig.add_subplot(gs[0,:])
+    table_ax.axis('off')
+    table_data = populate_table_rows(agg, all_nucs, times)


Is there a reason this can't just be a pivoted dataframe, instead of the list of lists that you generate? You could then use pandas.plotting.table() perhaps?

No reason other that I just didn't know about pandas.plotting.table() until now!

Full disclosure to reveal my through process: I didn't know about it either. I just had a hunch that once we had a dataframe in the right shape (which pivoting does), it should be possible to sort it easily and then guessed that it was possible to generate a "pretty" version in output, so went hunting/googling for that....

I just updated the PR with a bunch of changes, and after having messed around with the pandas.plotting.table() vs the matplotlib.pyplot.table(), I actually think that using the matplotlib version will work better because it allows for custom row and column labels, whereas the pandas version demanded the labels be the same as those from the input DataFrame, which would add unnecessary steps to arrive at the same end.

gonuke · 2025-12-02T12:59:58Z

tools/alara_output_processing/alara_output_plotting.py

+    row_labels = [reformat_isotope(nuc) for nuc in all_nucs]
+
+    table_data_float = np.array([
+        [float(cell.strip('%')) for cell in row] for row in table_data


It looks like you add the "%" when you process the data only to remove it here. Why not just put the float data in the table in the first place?

gonuke · 2025-12-02T21:36:29Z

tools/alara_output_processing/alara_output_plotting.py

@@ -87,7 +90,7 @@ def preprocess_data(

    return times, filtered, piv


In hindsight, it's not clear to me why return times since those times will be available in filtered (and piv). From a data integrity point of view, it always seems safest to extract the times from those dataframes rather than pass them around as a separate data object

gonuke · 2025-12-02T21:38:34Z

tools/alara_output_processing/alara_output_plotting.py

+
+    return times, agg
+
+def populate_table_rows(agg, all_nucs, times):


see comment below, but it's not clear why you need this method when you could just use a dataframe rather than a list of lists?

gonuke · 2025-12-03T01:14:34Z

tools/alara_output_processing/alara_output_plotting.py

+    # Create table gridspec
+    left = 0.05
+    top = 0.95
+    gs = fig.add_gridspec(
+        nrows + 1, ncols,
+        height_ratios=([1.0] + [1]*nrows),
+        hspace=0.4,
+        wspace=0.3,
+        left=left, right=(1-left), top=top, bottom=(1-top)
+    )


Separation of concerns... I think the GridSpec should be generated at a different level, and then the specific table_ax passed into this method to populate it.

gonuke · 2025-12-03T01:53:09Z

tools/alara_output_processing/alara_output_plotting.py

+        None
+    '''
+
+    times, agg = pie_chart_aggregation(


separation of concerns: could we have the user do this aggregation and then pass the result into the method to generate the pie chart?

gonuke · 2025-12-03T01:58:07Z

tools/alara_output_processing/alara_output_plotting.py

+        # Color in index
+        table[(i+1, -1)].set_facecolor(rgba)
+
+        # Color all other columns
+        for j in range(len(times)):
+            table[(i+1, j)].set_facecolor(rgba)


These can be combined into a single loop

Suggested change

# Color in index

table[(i+1, -1)].set_facecolor(rgba)

# Color all other columns

for j in range(len(times)):

table[(i+1, j)].set_facecolor(rgba)

# Color all columns in this row

for j in range(len(times)+1):

table[(i+1, j-1)].set_facecolor(rgba)

gonuke · 2025-12-03T01:58:37Z

tools/alara_output_processing/alara_output_plotting.py

+    table.scale(1.0, 2.0)
+
+    # Format table for readability
+    for i in range(len(row_labels)):


prefer row and col vs i and j... or at least r and c

gonuke · 2025-12-03T02:01:40Z

tools/alara_output_processing/alara_output_plotting.py

+    if np.any(fractions == 1.0):
+        for nuc, frac in zip(time_slice['nuclide'], fractions):
+            labels.append(reformat_isotope(nuc) if frac == 1.0 else '')
+        return labels


Isn't this edge case already covered by the case below, as long as you handle Other more carefully? Maybe the condition is:

if frac >= threshold or (nuc == 'Other' && frac > 0):

tools/alara_output_processing/alara_output_plotting.py

gonuke

Just a few more things to clarify...

gonuke · 2025-12-04T19:20:15Z

tools/alara_output_processing/alara_output_plotting.py


    return {lbl: cmap(i % cmap.N) for i, lbl in enumerate(sorted(all_nucs))}

+def build_color_map_from_nucs(all_nucs, cmap_name):


It's not obvious how this is substantially different from build_color_map_from_pivs?

gonuke · 2025-12-08T03:41:08Z

tools/alara_output_processing/alara_output_plotting.py

+        None
+    '''
+
+    table_ax = fig.add_subplot(gs[0,:])


I think it makes sense to do this before calling this method, and pass in table_ax. Then it's possible for someone to decide where in the gridspec if goes before calling this

gonuke · 2025-12-08T03:52:25Z

tools/alara_output_processing/alara_output_plotting.py

    plt.tight_layout(rect=[0, 0, 0.85, 1])
-    plt.show()
-
+    plt.close(fig)


Do we need to close the figure? Is it a scope issue... because we created the figure here in this function?

gonuke

A few small changes

gonuke · 2025-12-22T22:36:51Z

tools/alara_output_processing/alara_output_plotting.py

+    if np.array(all_nucs).size == 0:
+        if pivs:
+            all_nucs = set()
+            for piv in pivs:
+                all_nucs.update(piv.index)
+
+        else:
+            raise ValueError(
+                'Must input either all_nucs or pivs.'
+            )


Suggested change

if np.array(all_nucs).size == 0:

if pivs:

all_nucs = set()

for piv in pivs:

all_nucs.update(piv.index)

else:

raise ValueError(

'Must input either all_nucs or pivs.'

)

all_nucs = set(all_nucs)

if len(all_nucs) == 0:

for piv in pivs:

all_nucs.update(piv.index)

if len(all_nucs) == 0:

raise ValueError(

'Must input either all_nucs or pivs.'

)

gonuke · 2025-12-22T22:40:19Z

tools/alara_output_processing/alara_output_plotting.py

+    times = sorted(agg['time'].unique())
+    N = len(times)
+    nrows = int(np.ceil(N / ncols))
+    fig = plt.figure(figsize=(5 * ncols + 3, 5 * (nrows + 0.7)))


What are these magic numbers 5, 3, 0.7?

Simply formatting values that made the plot come out looking nice. Would you prefer they be optional parameters that the user can assign?

or give them some name that evokes some kind of meaning...

padding = 5 vspace = 3 hspace = 4.5

gonuke · 2025-12-22T22:46:22Z

tools/alara_output_processing/alara_output_plotting.py

+
+    return wedges
+
+def open_with_new_manager(fig):


Do we still need this function?

gonuke

LGTM - thanks @eitan-weinstein

gonuke requested changes Dec 3, 2025

View reviewed changes

gonuke requested changes Dec 8, 2025

View reviewed changes

gonuke requested changes Dec 22, 2025

View reviewed changes

Eitan Shai Weinstein and others added 23 commits December 23, 2025 15:09

Plotting tool and helper functions; basic implementation in Jupyter.

6ea36db

Making plotting function name more precise.

ae3e2c2

Restoring plots.

89c88ff

Adjusting to new demands of alara_output_processing

b4d013b

Removing extraneous cell

3aa689f

Single pie chart update for ADF

2e83aa9

Initial commit for pie chart grid

43baf9e

Comp tool refactored for new output parser.

8f4f26f

Plotting tool and helper functions; basic implementation in Jupyter.

99e64e8

Making plotting function name more precise.

3480f44

Restoring plots.

965c804

Adjusting to new demands of alara_output_processing

e45cdcc

Plotting tool and helper functions; basic implementation in Jupyter.

b454a0d

Making plotting function name more precise.

068c3ed

Restoring plots.

0677310

Including relative plotting functionality.

11a6157

Finishing rebase on Jupyter

90ac3d3

Refactoring for alara_output_processing changes

9b2f53d

Modifications for ADF updates

c1e75f4

Responding to requested changes.

c0b24ea

Missing save in notebook

9e0fda3

Fixing pie_chart_aggregation() docstring

a6070cb

Responding to requested changes.

1bea9d2

eitan-weinstein force-pushed the pie_chart_grid branch from 9ba9ddd to 1bea9d2 Compare December 23, 2025 21:10

Eitan Shai Weinstein added 2 commits December 23, 2025 15:13

Responding to requested changes.

ae4d23a

Making figure sizing user adjustable

56908ce

gonuke approved these changes Dec 29, 2025

View reviewed changes

gonuke merged commit dcc9657 into svalinn:main Dec 29, 2025

		@@ -87,7 +90,7 @@ def preprocess_data(

		return times, filtered, piv


		return times, agg

		def populate_table_rows(agg, all_nucs, times):


		return {lbl: cmap(i % cmap.N) for i, lbl in enumerate(sorted(all_nucs))}

		def build_color_map_from_nucs(all_nucs, cmap_name):

Conversation

eitan-weinstein commented Dec 2, 2025

Uh oh!

gonuke left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

gonuke left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

gonuke left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

eitan-weinstein Dec 23, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

gonuke left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

eitan-weinstein Dec 23, 2025 •

edited

Loading