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

Backscatter #64

Merged
merged 21 commits into from Feb 25, 2021
Merged

Backscatter #64

merged 21 commits into from Feb 25, 2021

Conversation

StJuricke
Copy link
Collaborator

Pull request for Backscatter branch

use_cavity=.false. !
use_cavity_partial_cell=.false.
!use_cavity=.false. !
!use_cavity_partial_cell=.false.
Copy link
Member

Choose a reason for hiding this comment

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

@StJuricke Can you, please, also uncomment those two lines?

src/oce_dyn.F90 Outdated
UV_rhs(1,nz,el(2))=UV_rhs(1,nz,el(2))+u1/elem_area(el(2))
UV_rhs(2,nz,el(1))=UV_rhs(2,nz,el(1))-v1/elem_area(el(1))
UV_rhs(2,nz,el(2))=UV_rhs(2,nz,el(2))+v1/elem_area(el(2))
UV_total_tend(1,nz,el(1))=UV_total_tend(1,nz,el(1))-u1/elem_area(el(1))
Copy link
Collaborator

Choose a reason for hiding this comment

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

@StJuricke if backscatter is off I would not do the lines with UV_*_tend
why to spend resources then? the same for allocation (not to allocate at all)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@dsidoren I also used the output of UV_*_tend for some of the classical viscosity options to see how the dissipation tendencies look there. It's just a general diagnostic so it could probably be switched on and off by one of the diagnostics switches we already have. Or you can leave it out for now and I just use it when I do intercomparison studies. Not sure if anyone else is generally interested in dissipation tendencies right now.

Copy link
Collaborator

Choose a reason for hiding this comment

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

@StJuricke having it under IF (BACKSTATTER) would be nice otherwise we compute and allocate something which is not used and is costly (3D elemental fields).

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@dsidoren I think BACKSCATTER as a switch would be missleading, as it is more a tendency diagnostic. So we could introduce a switch like momentum_diag or so. How should we proceed here? Should I for now just delete those lines and resubmit to github so that we can merge, and I will add this switch option later? Or can you delete those lines from your side?

Copy link
Member

Choose a reason for hiding this comment

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

@StJuricke It would be best if you could delete it on your side and introduce a diagnostic switch later on. It's good to have the opportunity of turning on additional diagnostic when they are needed. My understanding is @dsidoren is fine with the rest of the PR, so when you delete those variables, I will test and merge. It would be great to have another PR from you with additional diagnostics!

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Sorry, it seems like my post below was not published. So I will remove the diagnostics in my side and resubmit, right

Old reply to Dima's comment:
@dsidoren I think BACKSCATTER as a switch would be missleading, as it is more a tendency diagnostic. So we could introduce a switch like momentum_diag or so. How should we proceed here? Should I for now just delete those lines and resubmit to github so that we can merge, and I will add this switch option later? Or can you delete those lines from your side?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Oh, it was published... weird. Anyway, I will change it on my side

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@dsidoren @koldunovn I removed the extra diagnostics for all schemes except the dynamic backscatter. I can add them again at a later point with a specific momentum_diagnostics switch, so that they can be used by other (which I doubt many people, if any, will ever use) but that can wait for later

Copy link
Member

Choose a reason for hiding this comment

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

@StJuricke Thnaks a lot! I will try to run it with the new option and everything is fine merge. Sorry for slow reaction, but things here are just crazy as usual, as you well know :)

@koldunovn koldunovn merged commit e99820a into master Feb 25, 2021
WiltonLoch pushed a commit to WiltonLoch/fesom2 that referenced this pull request Feb 1, 2023
@suvarchal suvarchal deleted the Backscatter branch December 13, 2023 08:10
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.

None yet

3 participants