Skip to content

Conversation

@lkwinta
Copy link
Contributor

@lkwinta lkwinta commented Oct 26, 2025

This pull request introduces a comprehensive system for managing and displaying the download status of Geant4 simulation datasets in the application. It adds new React hooks and UI components for monitoring, controlling, and informing users about dataset downloads, integrates these features into the main simulation workflow, and updates simulation run logic to support dataset selection.

Dataset Download Management and UI Integration

  • Added useDatasetDownloadManager hook in Geant4DatasetDownloadManager.ts to track and control the download status of Geant4 datasets, including polling progress from the worker and updating UI state.
  • Introduced new UI components: Geant4Datasets for download controls and status display, and DatasetsInfoDialog modal for dataset descriptions and download info. [1] [2]
  • Integrated dataset download manager state and controls into the main app (WrapperApp.tsx) and simulation panel (RunSimulationForm.tsx), allowing users to start downloads and view progress. [1] [2] [3] [4]

Simulation Run Logic Updates

  • Updated simulation run hooks (UseRunGeant4LocalWorkerSimulation.tsx, UseRunRemoteWorkerSimulation.tsx) to accept and pass the selected Geant4DatasetsType, enabling simulations to utilize downloaded datasets. [1] [2] [3] [4]

Geant4 Worker and Progress Monitoring Enhancements

  • Added dataset progress polling API to Geant4Worker, enabling periodic updates of download status and progress to the UI.
  • Improved progress monitoring and error handling in worker modules, including filtering out irrelevant error messages and clarifying status parsing. [1] [2] [3] [4]

Codebase Cleanup

  • Removed unused message types and redundant code related to dataset download status handling. [1] [2]

These changes provide users with a clear, interactive way to manage Geant4 dataset downloads, improving simulation speed and reliability while keeping the workflow intuitive.

This comment was marked as outdated.

@lkwinta lkwinta changed the title Add option to download full datasets prior to simulation Draft: Add option to download full datasets prior to simulation Oct 26, 2025
@lkwinta lkwinta requested a review from Copilot November 2, 2025 19:22

This comment was marked as outdated.

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

Copilot reviewed 13 out of 13 changed files in this pull request and generated 7 comments.

Comments suppressed due to low confidence (1)

src/WrapperApp/components/Simulation/Geant4DatasetDownload.tsx:14

  • Unused import CircularProgress.
import {
	AccordionDetails,
	AccordionSummary,
	Box,
	Button,
	CircularProgress,
	LinearProgress,
	Typography,
	useTheme
} from '@mui/material';

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@lkwinta lkwinta changed the title Draft: Add option to download full datasets prior to simulation Add option to download full datasets prior to simulation Nov 2, 2025
@lkwinta lkwinta force-pushed the full-dataset-download branch from 41f16f6 to aea9774 Compare November 6, 2025 18:45
@lkwinta lkwinta requested a review from Copilot November 6, 2025 18:51
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

Copilot reviewed 14 out of 14 changed files in this pull request and generated 2 comments.

Comments suppressed due to low confidence (1)

src/WrapperApp/components/Simulation/RunSimulationForm.tsx:434

  • This use of variable 'inputFiles' always evaluates to true.
								{Object.keys(inputFiles ?? {}).map((fileName, index) => (

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@lkwinta lkwinta requested a review from Copilot November 6, 2025 23:09
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

Copilot reviewed 16 out of 16 changed files in this pull request and generated 5 comments.

Comments suppressed due to low confidence (6)

src/Geant4Worker/Geant4Worker.ts:171

  • Missing early return after error message when deps are already loaded. This allows the code to continue and potentially reload deps, which could cause issues. Add return; after the console.error statement.
		if (this.depsLoaded) {
			console.error('Deps already loaded');
		}

src/Geant4Worker/Geant4Worker.ts:189

  • Missing early return after error message when deps are already loaded. This allows the code to continue and potentially reload deps lazily, which could cause issues. Add return; after the console.error statement.
		if (this.depsLoaded) {
			console.error('Deps already loaded');
		}

src/WrapperApp/components/Simulation/Geant4DatasetDownload.tsx:5

  • Unused import HelpOutlineIcon.
import HelpOutlineIcon from '@mui/icons-material/HelpOutline';

src/WrapperApp/components/Simulation/Geant4DatasetDownload.tsx:15

  • Unused import Alert.
import {
	AccordionDetails,
	AccordionSummary,
	Alert,
	Box,
	Button,
	LinearProgress,
	Typography,
	useTheme
} from '@mui/material';

src/WrapperApp/components/Simulation/Geant4DatasetDownload.tsx:16

  • Unused import palette.
import { palette } from '@mui/system';

src/WrapperApp/components/Simulation/Geant4DatasetDownload.tsx:70

  • Unused function showDetailsModal.
function showDetailsModal() {}

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@grzanka
Copy link
Contributor

grzanka commented Nov 7, 2025

Something is weird here. During downloading of G4EMLOW after fetching ~500MB over the net, I saw that my Firefox on Ubuntu ate ~10GB of my RAM, effetively freezing my OS.

image

At downloading the full dataset is extremely RAM hungry, maybe we should implement some protection for the users having low amount of RAM. Imagine all the unsaved files that will be lost after filling up whole RAM by yaptide tab in the browser. Users will be enraged...

Few ideas:

  • stop downloading if only 2GB of RAM (or any other configureable amount) is left in the OS
  • present RAM usage (in the OS and/or by the yaptide) to the user during file download
  • check if we really need all datasets to be downloaded. One could inspect lazy load dump and see if that means we access files from all datasets.
  • investigate why 10GB of RAM is used if we download ~0.5GB ...

I am not sure what is doable and if the problem can be reproduced on other OSes and browser.

Something more complex: let the users select which physics list to use. Then based on that we can select which datafiles to download. I think I can easily identify which physics list needs which datafile. For example the most basic physics lists (EMPhysics) needs G4EMLOW.

@grzanka
Copy link
Contributor

grzanka commented Nov 9, 2025

@lkwinta can you rebase on top of the newest master ?

kmichalikk
kmichalikk previously approved these changes Nov 14, 2025
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

Copilot reviewed 17 out of 17 changed files in this pull request and generated 6 comments.

Comments suppressed due to low confidence (1)

src/WrapperApp/components/Simulation/RunSimulationForm.tsx:434

  • This use of variable 'inputFiles' always evaluates to true.
								{Object.keys(inputFiles ?? {}).map((fileName, index) => (

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

Copilot reviewed 18 out of 18 changed files in this pull request and generated 9 comments.

Comments suppressed due to low confidence (1)

src/WrapperApp/components/Simulation/RunSimulationForm.tsx:434

  • This use of variable 'inputFiles' always evaluates to true.
								{Object.keys(inputFiles ?? {}).map((fileName, index) => (

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copy link
Contributor

@grzanka grzanka left a comment

Choose a reason for hiding this comment

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

See

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

Copilot reviewed 18 out of 18 changed files in this pull request and generated 12 comments.

Comments suppressed due to low confidence (1)

src/Geant4Worker/Geant4Worker.ts:169

  • Inconsistent error handling: console.error is used at lines 168 and 184 for the same class in the same file, but at lines 164, 180, 196, and 207, errors are thrown. The loadDeps() and loadDepsLazy() methods now throw errors (lines 164, 180), which is more appropriate than the previous console.error approach. However, loadDeps() at line 168 still logs to console when deps are already loaded. Consider throwing an error here too for consistency.
		if (this.depsLoaded) {
			console.error('Deps already loaded');
		}

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@lkwinta
Copy link
Contributor Author

lkwinta commented Nov 16, 2025

@grzanka I have responded to copilot commends and published latest version on yap-dev

@grzanka grzanka enabled auto-merge November 16, 2025 16:00
@grzanka grzanka disabled auto-merge November 16, 2025 16:03
@grzanka grzanka enabled auto-merge November 16, 2025 16:03
@grzanka grzanka added this pull request to the merge queue Nov 16, 2025
Merged via the queue into master with commit 79c5ad1 Nov 16, 2025
11 checks passed
@grzanka grzanka deleted the full-dataset-download branch November 16, 2025 16:19
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.

4 participants