tidy3d icon indicating copy to clipboard operation
tidy3d copied to clipboard

Show cost for EME, HEAT, and MODE. Remove URL from EME webapi.

Open caseyflex opened this issue 1 year ago • 1 comments

Tested for HEAT. Can't really test for EME until python-webapi is updated (see PR there)

caseyflex avatar Apr 24 '24 09:04 caseyflex

Thanks! I've merged your webapi PR and redeployed the service - give it a try now!

However, what exactly was happening before? In principle if the webapi errors, a metadata pipeline is meant to be kicked off such that the estimate should always eventually appear. Was that not what you were seeing?

My other comments are a bit nit-picky, but it would be great to avoid some code duplication.

How about we define a SOLVER_NAME dict that links task_type to the name we want printed in log messages. Then we can have a single message here and also replace this with just solver_name = SOLVER_NAME[task_type]?

I see. When I called web.estimate_cost, I got an estimated cost of 0.0 and metadataStatus='success', even though the metadata pipeline is giving the correct estimated cost. I don't know if this is using the webapi or some other web service. This is actually still the case, so I'm not sure why the estimated cost is 0. It doesn't appear to be running the metadata pipeline in the estimate_cost function.

I see that now when I call web.run with verbose=True, it shows the correct behavior. estimated cost is 0 and metadataStatus is processing, then after a while estimated cost is correct and metadataStatus is processed. This must be because the metadataPipeline is working correctly.

I converted this to a draft, I need to test a bit more.

caseyflex avatar Apr 30 '24 15:04 caseyflex

@momchil-flex I unified the treatment a bit between the solvers. It looked like the non-FDTD solver status loop was just a subset of the FDTD solver status loop. The unified logic seems to work well, and makes it easier to implement stuff like EME progress bar in the future. Could you check if this looks ok? If so, the PR should be good to merge.

caseyflex avatar May 08 '24 10:05 caseyflex

Looks good, played around a bit and it worked well. Hopefully no major issues will come up. :)

momchil-flex avatar May 09 '24 23:05 momchil-flex