Skip to content

9 being able to work without outputfolder and saving nothing or all temporary #10

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.

Sign up for GitHub

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

Merged
Pablo1990 merged 5 commits into from
Feb 2, 2026
Merged

9 being able to work without outputfolder and saving nothing or all temporary #10

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
df8b191
Creating temporary folder for simulations
Pablo1990 Feb 2, 2026
efc75dc
Missing Temp folder
Pablo1990 Feb 2, 2026
eef5eb0
Updating gitingore
Pablo1990 Feb 2, 2026
f08c5fa
If filename does not exist nor image raise Excepction
Pablo1990 Feb 2, 2026
547654e
working with temp folder indtead
Pablo1990 Feb 2, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions .gitignore
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -194,3 +194,5 @@ Input
*.xlsx

src/pyVertexModel/_version.py

Temp/
67 changes: 51 additions & 16 deletions src/pyVertexModel/algorithm/vertexModel.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import logging
import os
import shutil
import tempfile
from abc import abstractmethod
from itertools import combinations
from os.path import exists
Expand Down Expand Up @@ -251,16 +252,17 @@ def __init__(self, set_option='wing_disc', c_set=None, create_output_folder=True
self.tr = 0
self.numStep = 1

def initialize(self, img_input=None):
def initialize(self, img_input=None):
"""
Initialize the geometry and the topology of the model.
:param img_input: Optional. Either a filename (str) or a numpy array containing the image.
If None, uses the filename from settings.
"""
filename = os.path.join(PROJECT_DIRECTORY, self.set.initial_filename_state)

if not os.path.exists(filename):
if not os.path.exists(filename) and img_input is None:
logging.error(f'File {filename} not found')
raise FileNotFoundError(f'File {filename} not found')
Comment on lines +255 to +265
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot Feb 2, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Potential issue when initial_filename_state is None.

When initial_filename_state is None (the new default), os.path.join(PROJECT_DIRECTORY, None) will convert None to the string 'None', resulting in a path like /path/to/project/None. This creates a confusing error message since the path won't exist.

Consider adding an early check for None:

🐛 Proposed fix to handle None gracefully 
     def initialize(self, img_input=None):
         """
         Initialize the geometry and the topology of the model.
         :param img_input: Optional. Either a filename (str) or a numpy array containing the image. 
                          If None, uses the filename from settings.
         """
+        if self.set.initial_filename_state is None and img_input is None:
+            raise ValueError("Either initial_filename_state must be set or img_input must be provided")
+
         filename = os.path.join(PROJECT_DIRECTORY, self.set.initial_filename_state)

         if not os.path.exists(filename) and img_input is None:
             logging.error(f'File {filename} not found')
             raise FileNotFoundError(f'File {filename} not found')
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
def initialize(self, img_input=None):
"""
Initialize the geometry and the topology of the model.
:param img_input: Optional. Either a filename (str) or a numpy array containing the image.
If None, uses the filename from settings.
"""
filename = os.path.join(PROJECT_DIRECTORY, self.set.initial_filename_state)
if not os.path.exists(filename):
if not os.path.exists(filename) and img_input is None:
logging.error(f'File {filename} not found')
raise FileNotFoundError(f'File {filename} not found')
def initialize(self, img_input=None):
"""
Initialize the geometry and the topology of the model.
:param img_input: Optional. Either a filename (str) or a numpy array containing the image.
If None, uses the filename from settings.
"""
if self.set.initial_filename_state is None and img_input is None:
raise ValueError("Either initial_filename_state must be set or img_input must be provided")
filename = os.path.join(PROJECT_DIRECTORY, self.set.initial_filename_state)
if not os.path.exists(filename) and img_input is None:
logging.error(f'File {filename} not found')
raise FileNotFoundError(f'File {filename} not found')
🧰 Tools
🪛 Ruff (0.14.14)

[warning] 265-265: Avoid specifying long messages outside the exception class

(TRY003)

🤖 Prompt for AI Agents 
In `@src/pyVertexModel/algorithm/vertexModel.py` around lines 255 - 265, The code
currently always builds filename with os.path.join(PROJECT_DIRECTORY,
self.set.initial_filename_state) which turns None into the string "None"; update
initialize to first check if img_input is None and
self.set.initial_filename_state is None and handle that case (either skip
file-based initialization or raise a clear ValueError), and only call
os.path.join when self.set.initial_filename_state is not None; ensure any error
log references the actual missing filename or explicitly states that
initial_filename_state is unset; adjust the FileNotFoundError path check
accordingly in initialize.


base, ext = os.path.splitext(filename)
if self.set.min_3d_neighbours is None:
Expand Down Expand Up @@ -434,9 +436,10 @@ def iterate_over_time(self):
updating measures, and checking for convergence.
:return:
"""
temp_dir = os.path.join(self.set.OutputFolder, 'images')
if not os.path.exists(temp_dir):
os.makedirs(temp_dir)
if self.set.OutputFolder is not None:
temp_dir = os.path.join(self.set.OutputFolder, 'images')
if not os.path.exists(temp_dir):
os.makedirs(temp_dir)

if self.set.Substrate == 1:
self.Dofs.GetDOFsSubstrate(self.geo, self.set)
Expand Down Expand Up @@ -620,16 +623,17 @@ def save_v_model_state(self, file_name=None):
:param file_name:
:return:
"""
# Create VTK files for the current state
self.geo.create_vtk_cell(self.set, self.numStep, 'Edges')
self.geo.create_vtk_cell(self.set, self.numStep, 'Cells')
temp_dir = os.path.join(self.set.OutputFolder, 'images')
screenshot(self, temp_dir)
# Save Data of the current step
if file_name is None:
save_state(self, os.path.join(self.set.OutputFolder, 'data_step_' + str(self.numStep) + '.pkl'))
else:
save_state(self, os.path.join(self.set.OutputFolder, file_name + '.pkl'))
if self.set.OutputFolder is not None:
# Create VTK files for the current state
self.geo.create_vtk_cell(self.set, self.numStep, 'Edges')
self.geo.create_vtk_cell(self.set, self.numStep, 'Cells')
temp_dir = os.path.join(self.set.OutputFolder, 'images')
screenshot(self, temp_dir)
# Save Data of the current step
if file_name is None:
save_state(self, os.path.join(self.set.OutputFolder, 'data_step_' + str(self.numStep) + '.pkl'))
else:
save_state(self, os.path.join(self.set.OutputFolder, file_name + '.pkl'))

def reset_noisy_parameters(self):
"""
Expand Down Expand Up @@ -1264,4 +1268,35 @@ def objective(lambdas):
self.geo.update_measures()
result = minimize(objective, method='TNC', x0=np.array([self.set.lambdaS1, self.set.lambdaS2]), options=options)
logger.info(f'Found lambdaS1: {result.x[0]}, lambdaS2: {result.x[1]}')
return result.x[0], result.x[1]
return result.x[0], result.x[1]

def create_temporary_folder(self):
"""
Create temporary folder where store every require file during the simulation.
Copy link

Copilot AI Feb 2, 2026

Choose a reason for hiding this comment

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

Spelling error: 'require' should be 'required'. The docstring should read 'Create temporary folder where every required file is stored during the simulation.'

Suggested change
Create temporary folder where store every require file during the simulation.
Create temporary folder where every required file is stored during the simulation.

Copilot uses AI. Check for mistakes.
:return:
"""
if self.set.OutputFolder is not None:
logger.warning('Output folder already exists, using the existing one.')
return self.set.OutputFolder

# Create Temp folder if it doesn't exist
if not os.path.exists(os.path.join(PROJECT_DIRECTORY, 'Temp')):
os.makedirs(os.path.join(PROJECT_DIRECTORY, 'Temp'))
logger.info(f'Created Temp folder at: {os.path.join(PROJECT_DIRECTORY, "Temp")}')

# Create temporary folder in PROJECT_DIRECTORY/Temp/
temp_dir = tempfile.mkdtemp(dir=os.path.join(PROJECT_DIRECTORY, 'Temp'))
self.set.OutputFolder = temp_dir
logger.info(f'Created temporary output folder at: {temp_dir}')

return temp_dir

def clean_temporary_folder(self):
"""
Clean temporary folder after the simulation.
:return:
"""
if self.set.OutputFolder is not None and 'Temp' in self.set.OutputFolder:
shutil.rmtree(self.set.OutputFolder)
logger.info(f'Removed temporary output folder at: {self.set.OutputFolder}')
self.set.OutputFolder = None
Comment on lines +1299 to +1302
Copy link

Copilot AI Feb 2, 2026

Choose a reason for hiding this comment

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

The logic for checking temporary folders using string matching with 'Temp' is fragile and could lead to unintended behavior. If a user creates an output folder with 'Temp' anywhere in its path (e.g., 'MyTempResults' or '/home/user/Temporary/output'), this method would incorrectly delete it. Consider using a more robust approach, such as storing a flag indicating whether the folder is temporary, or checking if the folder is specifically within the PROJECT_DIRECTORY/Temp directory.

Suggested change
if self.set.OutputFolder is not None and 'Temp' in self.set.OutputFolder:
shutil.rmtree(self.set.OutputFolder)
logger.info(f'Removed temporary output folder at: {self.set.OutputFolder}')
self.set.OutputFolder = None
if self.set.OutputFolder is None:
return
output_folder = os.path.abspath(self.set.OutputFolder)
temp_root = os.path.abspath(os.path.join(PROJECT_DIRECTORY, 'Temp'))
try:
is_within_temp = os.path.commonpath([output_folder, temp_root]) == temp_root
except ValueError:
# If paths are on different drives or otherwise incomparable, do not delete.
is_within_temp = False
if is_within_temp and output_folder != temp_root and os.path.isdir(output_folder):
shutil.rmtree(output_folder)
logger.info(f'Removed temporary output folder at: {self.set.OutputFolder}')
else:
logger.warning(f"Output folder '{self.set.OutputFolder}' not removed because it is not a managed temporary folder.")
self.set.OutputFolder = None

Copilot uses AI. Check for mistakes.
3 changes: 2 additions & 1 deletion src/pyVertexModel/geometry/geo.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -366,7 +366,8 @@ def init_reference_cell_values(self, c_set):

# Update lmin0 with the minimum value in lmin_values
self.update_lmin0()
self.update_lmin0(default_value=find_lmin0_equal_target_gr(self, c_set))
if '/Temp/' not in c_set.initial_filename_state:
Copy link

Copilot AI Feb 2, 2026

Choose a reason for hiding this comment

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

This condition attempts to check if the initial_filename_state contains '/Temp/', but it will raise a TypeError when initial_filename_state is None because the 'in' operator cannot be used with None. The logic should first check if initial_filename_state is not None before checking if it contains '/Temp/'. A safer approach would be: 'if c_set.initial_filename_state is not None and '/Temp/' not in c_set.initial_filename_state:'

Suggested change
if '/Temp/' not in c_set.initial_filename_state:
if c_set.initial_filename_state is not None and '/Temp/' not in c_set.initial_filename_state:

Copilot uses AI. Check for mistakes.
self.update_lmin0(default_value=find_lmin0_equal_target_gr(self, c_set))
Comment on lines +369 to +370
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot Feb 2, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

TypeError when initial_filename_state is None.

Since initial_filename_state now defaults to None (as changed in set.py), this condition will raise a TypeError when checking '/Temp/' not in None. The in operator doesn't work with None.

🐛 Proposed fix to handle None case 
-        if '/Temp/' not in c_set.initial_filename_state:
+        if c_set.initial_filename_state is not None and '/Temp/' not in c_set.initial_filename_state:
             self.update_lmin0(default_value=find_lmin0_equal_target_gr(self, c_set))
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
if '/Temp/' not in c_set.initial_filename_state:
self.update_lmin0(default_value=find_lmin0_equal_target_gr(self, c_set))
if c_set.initial_filename_state is not None and '/Temp/' not in c_set.initial_filename_state:
self.update_lmin0(default_value=find_lmin0_equal_target_gr(self, c_set))
🤖 Prompt for AI Agents 
In `@src/pyVertexModel/geometry/geo.py` around lines 369 - 370, The membership
check "if '/Temp/' not in c_set.initial_filename_state" can raise TypeError when
c_set.initial_filename_state is None; update the condition used before calling
update_lmin0 (and the default_value call to find_lmin0_equal_target_gr) to guard
against None by first checking truthiness of c_set.initial_filename_state (e.g.,
treat None the same as "does not contain '/Temp/'"); modify the conditional
around update_lmin0 accordingly so it safely handles
c_set.initial_filename_state == None and still calls
update_lmin0(default_value=find_lmin0_equal_target_gr(self, c_set)) when
appropriate.


# Update BarrierTri0
self.update_barrier_tri0()
Expand Down
23 changes: 14 additions & 9 deletions src/pyVertexModel/parameters/set.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ def __init__(self, mat_file=None):
self.export_images = True
self.cLineTension_external = None
self.Contractility_external = False
self.initial_filename_state = 'Input/wing_disc_150.mat'
self.initial_filename_state = None
self.delay_lateral_cables = 5.8
self.delay_purse_string = self.delay_lateral_cables
self.ref_A0 = None
Expand Down Expand Up @@ -191,14 +191,19 @@ def check_for_non_used_parameters(self):
self.Remodel_stiffness_wound = 2

def redirect_output(self):
os.makedirs(self.OutputFolder, exist_ok=True)
os.makedirs(self.OutputFolder + '/images', exist_ok=True)
handler = logging.FileHandler(os.path.join(self.OutputFolder, 'log.out'))
handler.setLevel(logging.DEBUG)
formatter = logging.Formatter('%(asctime)s - %(name)s - %(levelname)s - %(message)s')
handler.setFormatter(formatter)
logger.addHandler(handler)
logger.propagate = True
"""
Redirect the output to a log file in the output folder
:return:
"""
if self.OutputFolder is not None:
os.makedirs(self.OutputFolder, exist_ok=True)
os.makedirs(self.OutputFolder + '/images', exist_ok=True)
handler = logging.FileHandler(os.path.join(self.OutputFolder, 'log.out'))
handler.setLevel(logging.DEBUG)
formatter = logging.Formatter('%(asctime)s - %(name)s - %(levelname)s - %(message)s')
handler.setFormatter(formatter)
logger.addHandler(handler)
logger.propagate = True

def read_mat_file(self, mat_file):
for param in mat_file.dtype.fields:
Expand Down
Loading