Skip to content

Time-dependent with multiple timesteps#20

Draft
joewallwork wants to merge 14 commits into
mainfrom
xt221
Draft

Time-dependent with multiple timesteps#20
joewallwork wants to merge 14 commits into
mainfrom
xt221

Conversation

@joewallwork

Copy link
Copy Markdown
Owner

This PR mainly exists to provide feedback,

@joewallwork joewallwork left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

This looks great @acse-xt221, keep up the good work! I left some comments that will hopefully be useful.

Comment thread examples/burgers_n2n/config.py Outdated
@@ -0,0 +1,38 @@
from models.burgers_n2n import *

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

If this file is identical to examples/burgers/config.py, perhaps you could avoid duplicating all these lines by just having the single line from examples.burgers.config import *?

Comment thread examples/burgers_one2n/config.py Outdated
@@ -0,0 +1,38 @@
from models.burgers_one2n import *

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Same again here.

@@ -0,0 +1,43 @@
from nn_adapt.layout import NetLayoutBase

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Similarly, you could plausibly import from examples/burgers/network.py, unless you plan to change the network architecture.

Comment thread examples/burgers_one2n/network.py Outdated
@@ -0,0 +1,43 @@
from nn_adapt.layout import NetLayoutBase

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

And again.

Comment thread examples/makefile Outdated
MODEL = steady_turbine
NUM_TRAINING_CASES = 100
MODEL = burgers
NUM_TRAINING_CASES = 1

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

This is okay to start off with. However, further down the line I think it would be a good idea to use a larger number of cases.

Comment thread nn_adapt/solving_one2n.py Outdated
Comment on lines +146 to +147
q_plus = solver_obj_plus.solution
# J = config.get_qoi(refined_mesh[-1])(q_plus[-1])

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

I don't think you need these lines.

Comment thread nn_adapt/solving_one2n.py Outdated
Comment on lines +200 to +204
V_plus = adj_sol_plus[step].function_space()
fwd_sol_plg = Function(V_plus)
tm.prolong(out["forward"][step], fwd_sol_plg)
adj_sol_plg = Function(V_plus)
tm.prolong(out["adjoint"][step], adj_sol_plg)

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Yes, this looks right.

Comment thread nn_adapt/solving_one2n.py Outdated

# Evaluate errors
dwr += dwr_indicator(config, mesh, fwd_sol_plg, adj_error)
dwr_list.append(dwr_indicator(config, mesh, fwd_sol_plg, adj_error))

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Nice. So these can be used as the "target" data for each timestep.

Comment thread nn_adapt/solving_one2n.py Outdated
return out if retall else out["dwr"]


def dwr_indicator(config, mesh, q, q_star):

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Again, if this is unmodified then you can just import it from solving.py, to avoid duplication.

Comment thread examples/models/burgers_one2n.py Outdated
self._solver.solve()


class Solver_one2n(nn_adapt.solving.Solver):

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

If this is largely the same as the Solver class in examples.models.burgers then you could just define this to be a subclass of it. That way, you only need to write out the methods that are different and don't need to repeat the others.

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.

2 participants