Skip to content
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.

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

Add option for time constant ascii data boundary #5059

Merged

Conversation

gassmoeller
Copy link
Member

The last step to make #5025 work. Add an option to AsciiDataBoundary to not support time dependent boundary conditions. This removes the irrelevant input parameters when a time-dependence of the dataset is not supported or allowed. Also make the class a bit safer to use without time dependence by initializing all time-dependent members to invalid values and reorder a few lines to only be executed when time-dependence is actually switched on.

This PR also fixes a long-standing property of AsciiDataBoundary, which is that even if we specify exactly one file without placeholder for timestep so far the class has issued a warning that it couldnt find the next file and therefore continues with a constant boundary condition. This has annoyed me for a long time, because if one specific file is given in the input file, it is obviously implied by the user that the boundary condition is not changing over time, and this warning was more confusing than helpful. In this new version we only issue the warning if we had found several files, and then reach the end of the file series.

{
// no longer consider the problem time dependent from here on out
// this cancels all attempts to read files at the next time steps
time_dependent = false;
// Give warning if first processor
Copy link
Member

Choose a reason for hiding this comment

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

would it make sense to set other parameters to invalid here?

// if filename does not contain a placeholder for timestep, no time dependence
// do not issue a warning, the parameter file is specifying exactly one file.
if (create_filename (0, 0) == create_filename (1, 0))
end_time_dependence (false);
Copy link
Member

Choose a reason for hiding this comment

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

if the function only changes the bool, I would prefer to get rid of the bool argument and just set time_dependent=false; here.

@tjhei
Copy link
Member

tjhei commented Feb 9, 2023

that gets rid of a lot of these annoying warnings from the tests. 👍

Copy link
Member

@tjhei tjhei left a comment

Choose a reason for hiding this comment

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

Very nice, thank you for the update! Looks much cleaner this way, in my opinion.

@gassmoeller
Copy link
Member Author

Yes, I actually like this form more as well. To your comment:

would it make sense to set other parameters to invalid here?

I would prefer not to. They are not used anymore, but I could see some applications for why we want to keep their values. E.g. one could ask the model: how many data files did you find? Or: please when you reach the last data file reverse the direction and go backward again to model an inversion of a rift basin. None of that is implemented right now, but I think it would be useful to have the possibility.

@tjhei
Copy link
Member

tjhei commented Feb 9, 2023

I would prefer not to.

I agree. 👍

@gassmoeller gassmoeller merged commit f4d96a2 into geodynamics:main Feb 9, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants