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

Cumulative updates to HyperplaneArrangements.m2 #414

Merged
merged 3 commits into from
Apr 25, 2016

Conversation

denham0
Copy link
Contributor

@denham0 denham0 commented Apr 22, 2016

This includes a few bug fixes (base field dependency in Orlik-Solomon
code) and user suggestions.

This includes a few bug fixes (base field dependency in Orlik-Solomon
code) and user suggestions
@DanGrayson
Copy link
Member

One of the tests ran out of CPU time on my machine at "assert(pdim EPY A3 == 3)". Could you simplify it?

@DanGrayson
Copy link
Member

The test itself seems to be old, so maybe the bug fixes slowed it down.

computing pdim over ZZ[six variables] was too slow
@denham0
Copy link
Contributor Author

denham0 commented Apr 22, 2016

Hi, Dan.

Yes, it may have been computing over QQ[vars] before instead of ZZ[vars]. I
changed it accordingly. Do I have to issue another pull request? (I’m new to
git.)

best wishes,

/Graham

On Apr 22, 2016, at 3:10 PM, Daniel R. Grayson notifications@github.com wrote:

The test itself seems to be old, so maybe the bug fixes slowed it down.


You are receiving this because you authored the thread.
Reply to this email directly or view it on GitHub #414 (comment)

@DanGrayson
Copy link
Member

No, leave this pull request open, just push more commits to the branch of
yours associated with the pull request. You'll see a visible indication on
the pull request page that it has happened.

Run check "HyperplaneArrangements" to verify the tests.

On Fri, Apr 22, 2016 at 3:25 PM, denham0 notifications@github.com wrote:

Hi, Dan.

Yes, it may have been computing over QQ[vars] before instead of ZZ[vars]. I
changed it accordingly. Do I have to issue another pull request? (I’m new
to
git.)

best wishes,

/Graham

On Apr 22, 2016, at 3:10 PM, Daniel R. Grayson notifications@github.com
wrote:

The test itself seems to be old, so maybe the bug fixes slowed it down.


You are receiving this because you authored the thread.
Reply to this email directly or view it on GitHub <
https://github.com/Macaulay2/M2/pull/414#issuecomment-213557473>


You are receiving this because you commented.
Reply to this email directly or view it on GitHub
https://urldefense.proofpoint.com/v2/url?u=https-3A__github.com_Macaulay2_M2_pull_414-23issuecomment-2D213577137&d=CwMFaQ&c=8hUWFZcy2Z-Za5rBPlktOQ&r=PAZ_jTPLrSb1btCfAu4RkycWe57N0sGyzbjgeOn2wN0&m=DnmJEqfYWPqs3Fj4qkHN2QzvcIPywr6mX5VHPwM_WQU&s=X8KZ9QF7P9U-4QcBafD9Xk05UKeUB5WjJsBJFUf3hcE&e=

@DanGrayson
Copy link
Member

Can you submit the fix today? We'd like to finalize things for 1.9.

@denham0
Copy link
Contributor Author

denham0 commented Apr 25, 2016

I made a second commit three days ago that should fix the test issue. (So now it says this branch is 2 commits ahead.) "changed EPY test..." above. Is it available to you, or did I miss a step?

@DanGrayson
Copy link
Member

You must push your commits to github. Type

git push

@DanGrayson
Copy link
Member

Wait, now I see it on the list above! Maybe I missed seeing it or failed to refresh the page. I'll take a look .

@DanGrayson
Copy link
Member

Okay, now it fails with this error:

i22 : assert(multIdeal(2,X3) == multIdeal(2.2,X3))
stdio:27:27:(3):[1]: error: no method found for applying multIdeal to:
     argument 1 :  2.2 (of class RR)
     argument 2 :  {x , x , x , x  + x , x  + x . (of class CentralArrangement)
                     1   2   3   1    2   1    3.

The 2.2 looks suspicious.

Did you run check "HyperplaneArrangements"?

@DanGrayson
Copy link
Member

I should have mentioned: if you see this branch is 2 commits ahead then it means you have to push.

@denham0
Copy link
Contributor Author

denham0 commented Apr 25, 2016

definitely suspicious. I'll try again later this afternoon.

used RR before instead of QQ
@denham0
Copy link
Contributor Author

denham0 commented Apr 25, 2016

now it passes check "HyperplaneArrangements" so it should be good to go.

@DanGrayson DanGrayson merged commit 0681b94 into Macaulay2:master Apr 25, 2016
@DanGrayson
Copy link
Member

Okay, thanks, all done!

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