Skip to content

Added 'Charts' section for quick visualization of CSV outputs#1

Open
dihydrogenmonoxid-ama wants to merge 4 commits into
ProfRino:mainfrom
dihydrogenmonoxid-ama:main
Open

Added 'Charts' section for quick visualization of CSV outputs#1
dihydrogenmonoxid-ama wants to merge 4 commits into
ProfRino:mainfrom
dihydrogenmonoxid-ama:main

Conversation

@dihydrogenmonoxid-ama

Copy link
Copy Markdown

Dear Prof. Lovreglio,
First of all, thank you for your great work on this project! It has been incredibly useful.
I have implemented a new 'Charts' section to allow for a quick overview of automatically generated FDS simulation CSV files.

Key features of this update:

  • Data Visualization: Users can now visualize all DEVC and HRR measurement points directly within the browser.
  • Quick Plotting: Added functionality for quick, integrated plotting for an immediate "quick & dirty" check of the simulation results.
  • Documentation: Updated the 'Help' section to include instructions and context for these new charting features.
  • Note: This feature is intended for rapid data review during the simulation workflow. For scientifically rigorous post-processing, I will continue to use dedicated tools.
charts

I hope this contributes positively to the project. Any feedback or suggestions regarding this contribution would be greatly appreciated.
Best regards,
dihydrogenmonoxid

@chraibi chraibi left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@dihydrogenmonoxid-ama
Nice contribution — self-contained, zero-dependency, theme-aware, and consistent with the existing panels. A few things to address before merge:

Must fix

Drag-and-drop into Charts is broken. Help + CHANGELOG say you can drop CSVs into the Charts view, but charts-panel.js only wires the file <input>. The global handler in app.js intercepts the drop first:

const onDocDrop = (e) => { ...; loadFile(e.dataTransfer.files[0]); };  // → geometry loader, first file only

So a dropped CSV goes to the FDS geometry loader, not chartsPanelHandleFiles. Either implement DnD (gate onDocDrop on the active page and route .csv to charts) or drop the claim from Help/CHANGELOG.

Should fix

  • Dual-axis silently merges 3+ units. getActiveSeriesGrouped puts the first unit on the left and everything else on the right — so 3 distinct units share one right-hand scale labeled only Value (mixed). Misleading for a publication-style export. Warn the user, or refuse the 2nd axis when the right group is itself mixed.
  • No downsampling. _devc.csv files often have 10⁴–10⁵ rows; every render walks all points. Add a point-stride above ~5000 to keep redraw/resize smooth.
  • Scope: the PR also adds a .out log viewer + Linter button to the Code tab. That's a separate feature from "Charts" — consider splitting or retitling.

Nits

  • Version mismatch: CHANGELOG says charts-panel.js20260522D, file/index.html say 20260522E.
  • Squash the two identical commits.
  • Decimal-comma parse uses replace(',', '.') (first comma only) — 1.234,56 becomes 1.234.

@ProfRino

ProfRino commented Jun 7, 2026

Copy link
Copy Markdown
Owner

Thank you @dihydrogenmonoxid-ama very interesting expansion!
Can yoy please address the comment by @chraibi?
I would like to merge your feature
Thanks
Rino

@ProfRino

ProfRino commented Jun 7, 2026

Copy link
Copy Markdown
Owner

@dihydrogenmonoxid-ama I was reflecting on this new component.
Can you add it in the Output section as the charts are basically showing some of the FDS outputs?
Thanks
Rino

dihydrogenmonoxid-ama added 2 commits June 8, 2026 09:44
@dihydrogenmonoxid-ama

Copy link
Copy Markdown
Author

Thank you @chraibi for the thorough review, and thank you @ProfRino for your encouragement!

I have addressed all points in the latest commit:

  • Must fix — DnD routing: onDocDrop in app.js now routes dropped .csv files directly to chartsPanelHandleFiles instead of the geometry loader.
  • Structural: The Charts panel is no longer a standalone top-level tab — it is now a sub-mode of the Output section (alongside Soot / Slice / Boundary), which makes much more sense semantically.
  • Dual-axis warning: An amber warning badge is shown when the right Y-axis contains channels with more than one distinct unit.
  • Downsampling: Series with more than 5 000 points now use a stride to cap the rendered point count and keep redraws smooth.
  • Decimal-comma fix: Changed replace(',', '.') to replace(/,/g, '.') to correctly handle values like 1.234,56.
  • Version mismatch: Unified to 20260522E across CHANGELOG, file header, and index.html.

Please let me know if anything needs further adjustment. Happy to iterate!

screenshot-fds-viewer

@ProfRino

ProfRino commented Jun 8, 2026

Copy link
Copy Markdown
Owner

@dihydrogenmonoxid-ama thanks for trying to integrate the comment but please do not rush through the changes.

Please make the integration with Otput section more aligned with the other tabs.
← Back to Output should not be there as it is not there for Soot, Slice etc

The Open CSV file(s) should appear on the right column below Soot, Slice, Bouday, Charts

This solution current solution in not aligned to the existing naviagation style
image

The current version seem buggy too see the image here
image

Please take the time you need to make a nice integration of this new feature
Thanks
Rino

@chraibi

chraibi commented Jun 8, 2026

Copy link
Copy Markdown
Collaborator

@dihydrogenmonoxid-ama thanks for the quick reaction time ^^
DnD routing, the mixed-units warning, downsampling, and the decimal-comma fix all look good.

Three things still open before merge:

  1. Navigation alignment (per @ProfRino): Charts should behave like the other Output modes — drop the "← Back to
    Output" button, and move "Open CSV file(s)" into the right Data panel below Soot/Slice/Boundary/Charts rather
    than a separate left sidebar. The current layout also still looks buggy in Rino's screenshot.
  2. Scope: the .out log viewer + Linter button in the Code tab are a separate feature from Charts — please split
    them into their own PR.

Take your time on the integration. No rush and many thanks for your contribution!

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