search

clawpack / amrclaw

AMR version of Clawpack
http://www.clawpack.org
BSD 3-Clause "New" or "Revised" License
26 stars 45 forks source link

Switch `mstart` from being a local variable to the one stored in `amr_module` #254

Closed mandli closed 4 years ago

mandli commented 4 years ago

This appears to be an issue when the f90 refactor occurred. This fixes that.

mandli commented 4 years ago

Note that at this point I cannot get this to pass the tests but it looks like it has something to do with the adjoint tests and I am having difficulty understanding why.

mjberger commented 4 years ago

shoudln't have anything to do with it. There must a different thing going on - versionitis?

— Marsha

On Dec 17, 2019, at 6:07 PM, Kyle Mandli notifications@github.com wrote:

@mandli https://github.com/mandli requested your review on: #254 https://github.com/clawpack/amrclaw/pull/254 Switch mstart from being a local variable to the one stored in amr_module.

— You are receiving this because your review was requested. Reply to this email directly, view it on GitHub https://github.com/clawpack/amrclaw/pull/254?email_source=notifications&email_token=AAGUGC7QMUG62T6XZLBFGQ3QZFLSFA5CNFSM4J4CZPB2YY3PNVWWK3TUL52HS4DFWZEXG43VMVCXMZLOORHG65DJMZUWGYLUNFXW5KTDN5WW2ZLOORPWSZGOVRNUTJA#event-2891663780, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAGUGC3KZQOIRAF6NVU5UILQZFLSFANCNFSM4J4CZPBQ.

mandli commented 4 years ago

I agree and it looks like Travis is passing so I think this is probably safe to merge.

rjleveque commented 4 years ago

What test isn't passing? I tried this and it seems to work fine for me, both all the tests and examples/acoustics_1d_adjoint.

rjleveque commented 4 years ago

As far as I can see, the only place mstart is used in these subroutines is in call outtre(mstart,.false.,nvar,naux) immediately before stop in the case of an error, so I don't see this change can affect cases that are running properly.

mandli commented 4 years ago

I agree but it's better to have it fixed. I think my own testing is messed up right now as well so I would not worry about it if this looks good.