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

Better assy solver implementation #1063

Merged
merged 17 commits into from
May 18, 2022
Merged

Better assy solver implementation #1063

merged 17 commits into from
May 18, 2022

Conversation

adam-urbanczyk
Copy link
Member

@adam-urbanczyk adam-urbanczyk commented Apr 26, 2022

This PR implements a new assy solver based on Casadi (for autodiff) and IPOPT (actual optimizer, called via casadi).

@adam-urbanczyk adam-urbanczyk marked this pull request as draft April 26, 2022 06:57
@adam-urbanczyk
Copy link
Member Author

@lorenzncode not finished yet, but you could already take a look.

@adam-urbanczyk adam-urbanczyk changed the title Assy casadi Better assy solver implementation Apr 26, 2022
@codecov
Copy link

codecov bot commented Apr 26, 2022

Codecov Report

Merging #1063 (2988163) into master (803a05e) will increase coverage by 0.04%.
The diff coverage is 98.01%.

❗ Current head 2988163 differs from pull request most recent head 407bec5. Consider uploading reports for the commit 407bec5 to get more accurate results

@@            Coverage Diff             @@
##           master    #1063      +/-   ##
==========================================
+ Coverage   96.32%   96.37%   +0.04%     
==========================================
  Files          40       40              
  Lines        9427     9496      +69     
  Branches     1251     1256       +5     
==========================================
+ Hits         9081     9152      +71     
+ Misses        204      202       -2     
  Partials      142      142              
Impacted Files Coverage Δ
cadquery/assembly.py 96.55% <92.85%> (+1.50%) ⬆️
cadquery/occ_impl/solver.py 96.20% <98.27%> (+0.30%) ⬆️
cadquery/occ_impl/geom.py 99.58% <100.00%> (+<0.01%) ⬆️
tests/test_assembly.py 100.00% <100.00%> (ø)

📣 Codecov can now indicate which changes are the most critical in Pull Requests. Learn more

@lorenzncode
Copy link
Member

What a big effort the new implementation is.

I tested the constraints and did not find any significant problem.

  • FixedRotation: I would think gp_Extrinsic_XYZ might be a better default.

  • Could improve handling of a single unary constraint such as the following.

    assy.constrain("b1", "FixedRotation", (45,0,45))

    Currently it is allowed, however, ends with Number of Iterations....: 0

    The workaround is to add a dummy object and "Fixed" constraint. Of course a realistic assembly is expected to have more constraints but a new user might experiment like this. Either require a "Fixed" constraint or allow a single unary constraint without locking first entity.

@adam-urbanczyk
Copy link
Member Author

Thanks @lorenzncode . I changed to extrinsic and now it is explicitly disallowed to have only one unary constraint (or only one constrained entity to be exact).

@adam-urbanczyk adam-urbanczyk marked this pull request as ready for review May 12, 2022 05:41
cadquery/occ_impl/solver.py Show resolved Hide resolved
cadquery/occ_impl/solver.py Outdated Show resolved Hide resolved
cadquery/assembly.py Outdated Show resolved Hide resolved
cadquery/occ_impl/solver.py Outdated Show resolved Hide resolved
cadquery/occ_impl/solver.py Outdated Show resolved Hide resolved
conda/meta.yaml Show resolved Hide resolved
"nlp_scaling_method": "none",
"honor_original_bounds": "yes",
"bound_relax_factor": 0,
"print_level": 5,
Copy link
Member

Choose a reason for hiding this comment

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

The print output to stdout is useful. Perhaps an option can be exposed to reduce verbosity if the user chooses (not necessary for this PR can open a separate issue).

assy.solve(options={"verbose": 0})

(key "verbose", "verbosity", "print_level" or similar)

An options dict may be potentially useful to pass future options as well to control other algorithm settings or preferences (passed through to ConstraintSolver).

Copy link
Member Author

Choose a reason for hiding this comment

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

Let's open another issue for this. TBH I was thinking more in the direction of removing it in the near feature (once the solver is more established.

cadquery/occ_impl/solver.py Show resolved Hide resolved
@adam-urbanczyk
Copy link
Member Author

OK I think it is good enough now @lorenzncode @jmwright

@jmwright
Copy link
Member

+1 to merge @adam-urbanczyk @lorenzncode

Copy link
Member

@lorenzncode lorenzncode left a comment

Choose a reason for hiding this comment

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

Thanks @adam-urbanczyk!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

FixedRotation constraint doesn't fix rotation Simple square bar frame assembly doesn't line up
3 participants