Skip to content

Conversation

@AndreyAPopov
Copy link
Member

No description provided.

@AndreyAPopov AndreyAPopov changed the title Add tests for plot and movie Add tests for plot and movie and QG May 20, 2023
@Steven-Roberts Steven-Roberts mentioned this pull request Jun 8, 2023
@Steven-Roberts
Copy link
Member

I'm ok with merging this (maybe after #95). I haven't reviewed carefully, but I'll go though all the tests together carefully in the future

AndreyAPopov and others added 2 commits June 13, 2023 20:35
fixed typo for passing optional 'size' argument to the movie functions
@elswit
Copy link
Contributor

elswit commented Jun 23, 2023

Added fix for #91

model = otp.quasigeostrophic.presets.PopovMouIliescuSandu('size', [16, 32]);
model.TimeSpan = [0, 0.109];
catch
% if it failed to build, other tests would show it.
Copy link
Member

Choose a reason for hiding this comment

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

I think the try catch is unnecessary for this reason

model = eval(presetclass);
problem = eval(presetclass);
catch
continue;
Copy link
Member

Choose a reason for hiding this comment

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

Probably don't need try catch

try
problem = eval(presetclass);
catch
continue;
Copy link
Member

Choose a reason for hiding this comment

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

Same as above

end

try
problem.plot(sol);
Copy link
Member

Choose a reason for hiding this comment

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

fig = problem.plot(sol);
close(fig)

@Steven-Roberts
Copy link
Member

Steven-Roberts commented Oct 11, 2023

Add option to test plots but not movies in Octave. We can test what will happen on GitHub by running Octave without graphics. Hopefully the plots will display in the terminal!

@Steven-Roberts
Copy link
Member

TODO: fix formatting of PASS in table for some problems

@AndreyAPopov
Copy link
Member Author

I think this one is ready

@Steven-Roberts Steven-Roberts merged commit e170427 into master Jan 17, 2024
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