Open hyungyukang opened 1 day ago
Although personal testing indicates second-order convergence for 'Default,' 'Del2,' and 'Del4,' this must be validated with Polaris.
The above test was conducted on Frontier GPU using crayclanggpu.
@hyungyukang I have one small suggestion. Instead of clearing and re-creating default tendencies you could do this:
// these are empty by default
CustomTendencyType CustomThickTend;
CustomTendencyType CustomVelTend;
if (UseCustomTendency) {
// get manufactured tendencies etc
CustomThickTend = ManufacturedSol.ManufacturedThickTend;
CustomVelTend = ManufacturedSol.ManufacturedVelTend;
}
// one unconditional call to create
Tendencies::DefaultTendencies = create("Default", DefHorzMesh, NVertLevels, NTracers, &TendConfig, CustomThickTend, CustomVelTend);
@hyungyukang I have one small suggestion. Instead of clearing and re-creating default tendencies you could do this:
@mwarusz , thank you very much for the suggestion. I agree with you. I just tried it and it's working perfectly! I'm going to commit the code soon. May I include your name in the reviewer list if you are comfortable with it?
@hyungyukang
May I include your name in the reviewer list if you are comfortable with it?
Sure.
@xylar and @cbegeman ,
To enable this feature, Polaris needs to make some changes from config/Default.yml
.
Here is the difference between omega.yml
I used for the default manufactured solution test and config/Default.yml
.
@xylar and all. As I mentioned in the Error design, most of these errors should indeed be critical and abort, but we don't have a way of doing that easily until I get the new Error capability implemented (LOG_CRITICAL currently does not abort automatically). As part of that implementation, I'll be doing a pass through the code and replacing many of these error calls and error returns with critical errors and aborts when needed. But if we want to at least upgrade these to critical error messages, that will be a good hint about where we want this to happen.
And while I'm commenting on this PR...do we need to start thinking about multiple YAML configs now that we're starting to get conflicts. In particular, the streams unit test with this change will be writing output every 10 hours for a full year so probably not what we want.
@philipwjones, thanks for the update. I didn't realize that we weren't there yet with LOG_CRITICAL, sorry about that.
In particular, the streams unit test with this change will be writing output every 10 hours for a full year so probably not what we want.
I don't think @hyungyukang is proposing any changes to the YAML file (other than adding a few config options). These are the changes that Polaris will be making to the YAML file it uses (made manually for now).
Ah yes, I misread the comment on the Default.yml changes.
This PR adds custom tendency terms required for the manufactured solution test case to Omega. This feature can be tested in conjunction with the Polaris package, which has included a convergence study leveraging this capability. The details of this test case can be found in Section 2.10 and Figures 13 and 19 in Bishnu et al. 2024 and the manufactured solution test in Polaris.
The error norms for this test case are expected to demonstrate second-order convergence. The test results will be posted in this PR following integration with Polaris for this test case.
This PR does not modify the unit tests but passed them on Frontier using both CPU and GPU.
Checklist