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

Time-dependent with multiple timesteps #20

Draft
wants to merge 14 commits into
base: main
Choose a base branch
from
Draft

Time-dependent with multiple timesteps #20

wants to merge 14 commits into from

Conversation

jwallwork23
Copy link
Owner

This PR mainly exists to provide feedback,

Copy link
Owner Author

@jwallwork23 jwallwork23 left a comment

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.

@@ -0,0 +1,38 @@
from models.burgers_n2n import *
Copy link
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 *?

@@ -0,0 +1,38 @@
from models.burgers_one2n import *
Copy link
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
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.

@@ -0,0 +1,43 @@
from nn_adapt.layout import NetLayoutBase
Copy link
Owner Author

Choose a reason for hiding this comment

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

And again.

MODEL = steady_turbine
NUM_TRAINING_CASES = 100
MODEL = burgers
NUM_TRAINING_CASES = 1
Copy link
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 on lines 146 to 147
q_plus = solver_obj_plus.solution
# J = config.get_qoi(refined_mesh[-1])(q_plus[-1])
Copy link
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 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
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.


# 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
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.

return out if retall else out["dwr"]


def dwr_indicator(config, mesh, q, q_star):
Copy link
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.

self._solver.solve()


class Solver_one2n(nn_adapt.solving.Solver):
Copy link
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