Skip to content

Conversation

@dominiklaux
Copy link
Collaborator

This adds a trigger implementation to change the simulation resolution (perimeterResolution, spatialIncrement) during a simulation.

@dominiklaux dominiklaux self-assigned this May 30, 2025
@agumartina
Copy link

Well done @dominiklaux :)

{
double newPerimRes = getFloat("perimeterResolution", arg);
double newSpatialInc = getFloat("spatialIncrement", arg);

Choose a reason for hiding this comment

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

What if arg is not a float?
Maybe we can add somehting like:

 bool valid = true;
    if (newPerimRes != FLOATERROR) {
        if (newPerimRes <= 0.0 || newPerimRes > 10000.0 || std::isnan(newPerimRes)) {
            cout << "Error: perimeterResolution must be between 0 and 10,000." << endl;
            valid = false;
        }
    }
    if (newSpatialInc != FLOATERROR) {
        if (newSpatialInc <= 0.0 || newSpatialInc > 10000.0 || std::isnan(newSpatialInc)) {
            cout << "Error: spatialIncrement must be between 0 and 10,000." << endl;
            valid = false;
        }
    }

    if (!valid) return error;

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Thanks!
if arg is not a float, the getFloat() will return FLOATERROR, so this should be covered!
I added a check to ensure >= 0.0, but would rather avoid to set an upper limit to keep it simple?

@dominiklaux dominiklaux force-pushed the trigger-resolution branch from 0e4677d to 5d3698e Compare June 3, 2025 09:12
@agumartina
Copy link

Thanks for taking in consideration my review :)
Its a go for me, well done.

@dominiklaux dominiklaux merged commit a2b2fd3 into master Jun 4, 2025
2 checks passed
@dominiklaux dominiklaux deleted the trigger-resolution branch June 4, 2025 09:08
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.

2 participants